Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Failed Analysis tasks prevent Expr updates #4050

Closed
StachuDotNet opened this issue May 31, 2022 · 3 comments
Closed

Failed Analysis tasks prevent Expr updates #4050

StachuDotNet opened this issue May 31, 2022 · 3 comments
Labels
bug An actual bug

Comments

@StachuDotNet
Copy link
Member

StachuDotNet commented May 31, 2022

Right now, Blazor isn't serializing NodaTime.LocalDateTime in a way that the client understands.

As a result of this, creating a Repl with the simple expr of Date::now results in a Rollbar.
While we have an issue (#4007) to resolve that, working on such surfaced a bug:

If you set the expr to something that breaks in this way (again, Date::now works), and then attempt to update the expression to something that should work (e.g. 2+3), the issue is unresolved.

Without refreshing the browser, I suppose that the bug occurs because we have a BlazorWorker task where we haven't yet got a successful result. (so, one todo here may be to 'resolve' such tasks when they fail, so other analysis tasks may be performed.)

However, it surprisingly continues to fail even after a refresh. I've no idea how, but my Repl is now stuck. 2+3 seems to be the expr (visually), but I keep seeing the NodaTime error.

Video quality here is poor, but a demo:
ezgif-2-ae840fecb0

@StachuDotNet StachuDotNet added the bug An actual bug label May 31, 2022
@StachuDotNet
Copy link
Member Author

Currently focused on #4007. @pbiggar if you have any thoughts here, they'd be appreciated - otherwise I'll take a look at this after that.

@StachuDotNet
Copy link
Member Author

I was just able to reproduce this:

First, let's create a problem deserializing some payload from JS world:

  • edit Prelude.fs - create a Vanilla2 module below the Vanilla module - it should be the same, but not add the LocalDateTimeConverter()
    (we can't just edit Vanilla - other things break if we remove this converter generally)
  • edit Wasm.fs to use Vanilla2 for deserialization

Next, create a REPL with some code that will fail - its body should be Date::now. Upon playing the analysis, we'll get an issue deserializing, as expected.

Now, update the REPL to something that should work - 2+3

Adjust such will perform another analysis request - this fails. Here's the request:

[
  "AnalyzeHandler",
  {
    "handler": {
      "tlid": "1735861713",
      "spec": {
        "name": [ "Filled", "406747915", "abnormalCamel" ],
        "module": [ "Filled", "563542157", "REPL" ],
        "modifier": [ "Filled", "1783919258", "_" ],
        "types": {
          "input": [ "Blank", "577768209" ],
          "output": [ "Blank", "575346193" ]
        }
      },
      "ast": [
        "EBinOp",
        "1761700951",
        "+",
        [ "EInteger", "291665920", "2" ],
        [ "EInteger", "1828643484", "3" ],
        [ "NoRail" ]
      ]
    },
    "trace_id": "e57cec4e-c814-51dc-a87d-26021b1a8477",
    "trace_data": {
      "input": [],
      "timestamp": "1970-01-01T00:00:00Z",
      "function_results": [
        [
          "Date::now",
          "2117916793",
          "ViEJsFTLRCj8U2B7EHwKzfkd5DS5kKApX1Fu8qrXnvyQIjiUTnbULyG89xC_pMVU",
          1,
          [ "DDate", "2022-06-05T17:43:02Z" ]
        ]
      ]
    },
    "dbs": [],
    "user_fns": [],
    "user_tipes": [],
    "secrets": []
  }
]

The problem is that our previous Date::now result is being passed along. I just wasn't expecting it to hang around, causing the issue even when code is changed.

The issue here is sort of expected, and causes a rollbar, so not considering this a blocker, and deprioritizing for now.

@StachuDotNet
Copy link
Member Author

Closing, as this is just a specific case in the expectation of "if we can't deserialize data, errors happen," and not a more interesting/specific problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An actual bug
Projects
None yet
Development

No branches or pull requests

1 participant