roc-lang / basic-webserver Goto Github PK

Some reason, we use I128 for each of the values in this struct:

This feels exceptionally large for no reason. We should analyze what is required and minimize the waste here.

Currently we do a lot of allocating when going from rust to roc. I think we can avoid all of this on both sides.

I don't fully understand all of the pieces, but here is roughly how it would work.

Firstly, for all data being passed into roc, we will use seamless slices. This will make it so that the data does not need to be copied into roc data structures. Instead, roc will just reference the data and will never have to free it. That will make it live the entire time of the request though. So that is a tradeoff to think about. Technically more complex schemes could be used to avoid that.

Second, We need to implement the Body and Buf trait on RocList type. That will enable us to hand the roc list off to hyper with hyper reading directly from the list. Again, no need to copy data or reallocate. The implementation should be pretty simple and the same as would be done for a vector of bytes in rust.

The following tests will fail with the current handling of converting millis to DateTime:

# test last day of 1st year after epoch
expect 
    str = epochMillisToDateTime (364 * 24 * 60 * 60 * 1000) |> toIso8601Str
    str == "1970-12-31T00:00.00Z"

# test last day of 1st month after epoch
expect
    str = epochMillisToDateTime (30 * 24 * 60 * 60 * 1000) |> toIso8601Str
    str == "1970-01-31T00:00.00Z"

Bug is due to use of >= in current.day > countDaysInMonth and current.day > countDaysInYear. Should be >.

Utc does not currently support dates prior to epoch

This means that simple integer comparison with any date prior to 1970 is impossible using Utc. This also means that any library which attempts to parse ISO 8601 date strings to the Utc type will not be able to support the full range of ISO standard dates. Converting Utc from unsigned to signed integer will resolve this issue.

Opening an issue and PR for this as discussed on Zulip chat.

❯ ./roc build ~/gitrepos/basic-webserver/examples/todos.roc 
🔨 Rebuilding platform...
An internal compiler expectation was broken.
This is definitely a compiler bug.
Please file an issue here: https://github.com/roc-lang/roc/issues/new/choose
thread 'main' panicked at 'Undefined Symbol in relocation, (+1b97, Relocation { kind: PltRelative, encoding: Generic, size: +20, target: Symbol(SymbolIndex(+13d)), addend: +fffffffffffffffc, implicit_addend: false }): Ok(Symbol { name: "memset", address: +0, size: +0, kind: Label, section: Undefined, scope: Unknown, weak: false, flags: Elf { st_info: +10, st_other: +0 } })', crates/linker/src/elf.rs:1486:25
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

This is not specific to this example, and appears to happen for all of them.

The cause of this issue is: roc-lang/roc#3609

I think this only happens with export CARGO_BUILD_TARGET=x86_64-unknown-linux-musl. This is required for releases with good compatibility. I was also on the more-features branch, not on main.

PR #6395 removed Str.appendScalar from builtins. The code should be updated to user roc-lang/unicode instead.

ISO 8601 says that time should be in the format: hh:mm:ss or hhmmss. A comma or decimal point, for fractions of an hour, minute, or second may be added, but a decimal point should always indicate a fractional amount of the preceding time unit, such as:

• hh.hh
• hh:mm.mm
• hh:mm:ss.ss

In InternalDateTime, the toIso8601Str produces a result of the format YYYY-MM-DDThh:mm.ssZ, which if interpreted by any ISO conformant library, would be interpreted as YYYY-MM-DDThh:mm.mmZ, which of course is not the time represented.

Thus toIso8601Str should be changed to be conformant to the 8601 standard, and format as: YYYY-MM-DDThh:mm:ssZ.

When running the "helloweb" example webserver from the readme.md file (updated to use the 0.2.0 release), the webserver runs successfully, but as soon as a request is made to the server by loading 127.0.0.1:8000 in a browser, the application crashes with the following message (note that no backtrace is printed):

thread 'tokio-runtime-worker' panicked at 'The Roc app crashed with: Integer multiplication overflowed!', src/lib.rs:46:5
stack backtrace:

Note that I am using Roc version: roc_nightly-macos_apple_silicon-2023-12-29-4569770c82c

See:

• https://github.com/roc-lang/basic-webserver?tab=readme-ov-file#example
• https://github.com/roc-lang/basic-webserver/blob/main/.github/workflows/ci.yml

The example should be extracted from the README and put into a roc file for the test.

Url.roc for example contains a lot of shared code with Url.roc in basic-cli/platform. All shared code should be put in (probably multiple) Roc packages so that improvements don't need to be manually transferred between basic-webserver and basic-cli.

I tried to containerize a simple webapp, but was having issues with the host being hard coded to 127.0.0.1.

I resolved the issue by adding an environment variable ROC_BASIC_WEBSERVER_HOST that I set to 0.0.0.0 and used it like:

    let host = env::var(HOST_ENV_NAME).unwrap_or("127.0.0.1".to_string());
    let addr = format!("{}:{}", host, port).parse::<SocketAddr>().expect("Failed to parse host and port");

If this is an acceptable solution I'll make a PR, and I could add some more variables like PORT.

Otherwise, I'd love to hear it :)

When I run this (with latest source or 0.1 release)

app "repro"
    packages {
        # pf: "../basic-webserver/platform/main.roc",
        pf: "https://github.com/roc-lang/basic-webserver/releases/download/0.1/dCL3KsovvV-8A5D_W_0X_abynkcRcoAngsgF0xtvQsk.tar.br",
    }
    imports [
        pf.Task.{ Task },
        pf.Stdout,

        # Unused but needed to build
        pf.Http,
    ]

    provides [main] to pf

main = \_ ->
    when A is
        A ->
            {} <- Stdout.line "" |> Task.await
            respond

        _ ->
            respond

respond =
    Task.ok {
        status: 200,
        headers: [],
        body: [],
    }

I get this

$ RUST_BACKTRACE=full roc build repro.roc

── UNUSED IMPORT ─────────────────────────────────────────────────── repro.roc ─

Nothing from Http is used in this module.

11│          pf.Http,
             ^^^^^^^

Since Http isn't used, you don't need to import it.

────────────────────────────────────────────────────────────────────────────────

thread 'main' panicked at 'Error in alias analysis: error in module ModName("UserApp"), function definition FuncName("\x12\x00\x00\x00\x02\x00\x00\x00B\xf6\xc1\x05.\xbf\xec\xc7"), definition of value binding ValueId(3): could not find func in module ModName("UserApp") with name FuncName("%\x00\x00\x00\x04\x00\x00\x00B\xf6\xc1\x05.\xbf\xec\xc7")', crates/compiler/gen_llvm/src/llvm/build.rs:5743:19
stack backtrace:
   0:     0x555b894dded3 - <unknown>
   1:     0x555b8884717f - <unknown>
   2:     0x555b894d98a7 - <unknown>
   3:     0x555b894ddca5 - <unknown>
   4:     0x555b894df860 - <unknown>
   5:     0x555b894df624 - <unknown>
   6:     0x555b894dfd69 - <unknown>
   7:     0x555b894dfc74 - <unknown>
   8:     0x555b894de526 - <unknown>
   9:     0x555b894df9f2 - <unknown>
  10:     0x555b88746ea3 - <unknown>
  11:     0x555b88d7ee6c - <unknown>
  12:     0x555b88d78068 - <unknown>
  13:     0x555b88a43ccf - <unknown>
  14:     0x555b88a48319 - <unknown>
  15:     0x555b88a4705c - <unknown>
  16:     0x555b88b33d40 - <unknown>
  17:     0x555b88a15949 - <unknown>
  18:     0x555b88a094d3 - <unknown>
  19:     0x555b88a094f3 - <unknown>
  20:     0x555b894d0a45 - <unknown>
  21:     0x555b88a17d05 - <unknown>
  22:     0x7fb2b4229d90 - __libc_start_call_main
                               at ./csu/../sysdeps/nptl/libc_start_call_main.h:58:16
  23:     0x7fb2b4229e40 - __libc_start_main_impl
                               at ./csu/../csu/libc-start.c:392:3
  24:     0x555b887dbe3e - <unknown>
  25:                0x0 - <unknown>

But if I remove the stdout line

@@ -16,7 +16,7 @@ app "repro"
 main = \_ ->
     when A is
         A ->
-            {} <- Stdout.line "" |> Task.await
+            # {} <- Stdout.line "" |> Task.await
             respond

         _ ->

or if I inline one of the task (which one doesn't matter)

@@ -20,7 +20,11 @@ main = \_ ->
             respond

         _ ->
-            respond
+            Task.ok {
+                status: 200,
+                headers: [],
+                body: [],
+            }

it compiles.

This is the minimal example I can come up with. Not sure if issues belongs here or for the compiler, but I haven't been able to repro with the basic-cli platform.

I am on ubuntu 22

I discovered a bug in my own code, which was heavily influenced by epochMillisToDateTime (a.k.a I copy pasted it as a wip).
Adding this expect to InternalDateTime let's it crash:

# 2023-12-31 18:00:00
expect
    str =
        1_704_045_600_000
        |> epochMillisToDateTime
        |> toIso8601Str

    str == "2023-12-31T18:00.00Z"

> roc test ./platform/InternalDateTime.roc
── EXPECT PANICKED in ./platform/InternalDateTime.roc ──────────────────────────

This expectation crashed while running:

145│>  # 2023-12-31 18:00:00
146│>  expect
147│>      str =
148│>          1704045600_000
149│>          |> epochMillisToDateTime
150│>          |> toIso8601Str
151│>
152│>      str == "2023-12-31T18:00.00Z"

The crash reported this message:

Integer multiplication overflowed!

1 failed and 24 passed in 535 ms.

it seems that the error occurs here:

    if current.day >= countDaysInYear then
        epochMillisToDateTimeHelp {
            year: current.year + 1,
            month: current.month,
            day: current.day - countDaysInYear,
            # at some point current.hours = 8755
            # while (countDaysInYear * 24) = 8760
            # so:  current.hours - (countDaysInYear * 24) == -5
            hours: current.hours - (countDaysInYear * 24),
            minutes: current.minutes - (countDaysInYear * 24 * 60),
            seconds: current.seconds - (countDaysInYear * 24 * 60 * 60),
        }

When I find the time to fix it, I'll create a MR.