enso icon indicating copy to clipboard operation
enso copied to clipboard

Execute yjs from insight script

Open 4e6 opened this issue 1 year ago โ€ข 10 comments

Pull Request Description

Allow populating Yjs metadata with values provided by the insight script.

Compile the Insight Script

Run pnpm install

corepack pnpm i

Build app/ydoc-server/polyglot/insight.js bundle with

corepack pnpm run compile

Running

Start the project manager specifying the insight script location with ENSO_JVM_OPTS env variable

ENSO_JVM_OPTS="-Denso.dev.insight=$PWD/app/ydoc-server-polyglot/dist/insight.js"

Every time the insight.js is rebuilt, the engine reloads it automatically.

Debugging (in Chrome Dev Tools)

Add -Dpolyglot.inspect argument:

ENSO_JVM_OPTS="-Denso.dev.insight=$PWD/app/ydoc-server-polyglot/dist/insight.js -Dpolyglot.inspect"

The Engine stops and waits for Chrome Dev Tools to connect to it just like it waits for node.js processes.

debug in chrome dev tools

Don't Suspend

By default the Chrome Dev Tools integration stops on first executed JavaScript statement. That may not be appropriate in all situations. Add -Dpolyglot.inspect.Suspend=false to the ENSO_JVM_OPTS to disable such behavior. The engine still connects to Chrome Dev Tools, but stops only on breakpoints or when pause is explicitly requested in the debugger.

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • [ ] The documentation has been updated, if necessary.
  • [ ] Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • [ ] All code follows the Scala, Java, TypeScript, and Rust style guides. In case you are using a language not listed above, follow the Rust style guide.
  • [ ] Unit tests have been written where possible.
  • [ ] If meaningful changes were made to logic or tests affecting Enso Cloud integration in the libraries, or the Snowflake database integration, a run of the Extra Tests has been scheduled.
    • If applicable, it is suggested to paste a link to a successful run of the Extra Tests.

4e6 avatar Jan 02 '25 18:01 4e6

That's working:

  • parse the module contents into AST
  • attach node values provided by the insight as an AST metadata

What's in progress:

  • synchronize module AST with the ydoc-server. There is an issue with a WebSocket connection that may be caused by the context (mis)configuration

4e6 avatar Jan 02 '25 18:01 4e6

That's working:

* parse the module contents into AST

* attach node values provided by the insight as an AST metadata

What's in progress:

* synchronize module AST with the ydoc-server. There is an issue with a WebSocket connection that may be caused by the context (mis)configuration
  • What exactly is broken?
  • How can I use this PR to reproduce the failure?
    • what should I execute?
    • what should I see then?
  • can you resolve the merge conflicts, so I can try to play with an update to date working PR?

JaroslavTulach avatar Jan 31 '25 06:01 JaroslavTulach

@JaroslavTulach there is an issue with WebSocket connections from inside the insight script. You can reproduce it by trying to establish any connection, i.e.

let ws  = new WebSocket('wss://echo.websocket.org')

4e6 avatar Mar 10 '25 13:03 4e6

The Chrome Dev Tools debugging shows an exception when creating new socket:

exception

enso$ ./run backend sbt
sbt:enso> runProjectManagerDistribution 
  --env ENSO_JVM_OPTS=-Denso.dev.insight=$PWD/app/ydoc-server-polyglot/dist/insight.js
  --env ENSO_JVM_OPTS=-Dpolyglot.inspect

Concatenate the last three lines to a single sbt line command.

JaroslavTulach avatar Mar 13 '25 09:03 JaroslavTulach

@JaroslavTulach there is an issue with WebSocket connections from inside the insight script. You can reproduce it by trying to establish any connection,

This is the testing wstest.js to use:

var counter = 0;
function app() {
    let ws  = new WebSocket('wss://echo.websocket.org');
    function sayHi() {
      ws.send(`Hi there! #${counter++}`);
    }
    ws.onopen = function(ev) {
      print("Open: " + ev);
    };
    ws.onmessage = function(ev) {
      print("Msg   : " + Object.getOwnPropertyNames(ev));
      print("  data: " + ev.data);
      setTimeout(sayHi, 3000);
    };
    ws.onclose = function(ev) {
      print("Close: " + Object.getOwnPropertyNames(ev));
    };
    ws.onerror = function(ev) {
      print("Err: " + Object.getOwnPropertyNames(ev));
    };
}

if (typeof insight === "undefined") {
    globalThis.print = console.log;
    if (typeof WebSocket === "undefined") {
        let r = require("ws");
        globalThis.WebSocket = r.WebSocket;
    }
    app();
} else {
    app();
}

the testing script can by used from node (npm install ws and node wstest.js) or as an "Insight Script" in the GUI as ENSO_JVM_OPTS="-Denso.dev.insight=$(pwd)/wstest.js. The script is supposed to send a message to the echo server every three seconds. The script works fine in node js.

I can confirm, the connection gets randomly broken in the GUI. The cause is multithreadedness. GraalVM JavaScript isn't ready to be used from multiple threads. References:

  • https://github.com/oracle/graal/issues/8033
  • https://github.com/oracle/graal/pull/8266
  • #5176

We need to use better Executor to execute callbacks from WebSocket implementation "linearly" and not in parallel.

Update on Apr 11, 2025:

  • since #12528
  • the here-in provided script is working properly

JaroslavTulach avatar Mar 13 '25 11:03 JaroslavTulach

We need to use better Executor to execute callbacks from WebSocket implementation "linearly" and not in parallel.

  • Or we need to "recover from parallel Graal.js execution"
  • e.g. we need threadAccessDeniedHandler as introduced by oracle/graal#8266
  • hence #12500 - to get that new API
  • given there are plans to go multi threaded - #10525
  • possibly using JavaScript on multiple threads
  • we need something like threadAccessDeniedHandler to orchestrate and co-ordinate the access to JavaScript to happen a single thread at a time

JaroslavTulach avatar Mar 14 '25 03:03 JaroslavTulach

DAP Debugging

To debug via DAP use:

ENSO_JVM_OPTS="-Denso.dev.insight=$(pwd)/test.js -Dpolyglot.dap=4711" sbt runProjectManagerDistribution

and once the engine is starting (and blocks waiting for a connection) choose:

obrazek

and breakpoints in JavaScript are going to be hit.

JaroslavTulach avatar Mar 31 '25 05:03 JaroslavTulach

With the latest 048bd5c commit I am seeing:

enso$ ENSO_JVM_OPTS="-Denso.dev.insight=$(pwd)/app/ydoc-server-polyglot/dist/insight.js -Dpolyglot.dap=4711" sbt
sbt:enso> runProjectManagerDistribution
...
[Graal DAP] Starting server and listening on localhost/127.0.0.1:4711
[Graal DAP] Client connected on /127.0.0.1:60276
Initializing Insight: {"id":"insight","version":"1.2"}
attachProvider ws://[::1]:5976/project index
[WARN] [2025-04-11T16:42:42.629] [org.enso.languageserver.protocol.json.JsonConnectionController] Failed to initialize resources
java.util.concurrent.CompletionException: org.graalvm.polyglot.PolyglotException: java.net.ConnectException: Connection refused
        at java.base/java.util.concurrent.CompletableFuture.encodeThrowable(CompletableFuture.java:315)
        at java.base/java.util.concurrent.CompletableFuture.completeThrowable(CompletableFuture.java:320)
        at java.base/java.util.concurrent.CompletableFuture$AsyncRun.run(CompletableFuture.java:1807)
        at org.enso.akka.wrapper/akka.dispatch.TaskInvocation.run(AbstractDispatcher.scala:49)
        at java.base/java.util.concurrent.ForkJoinTask$RunnableExecuteAction.exec(ForkJoinTask.java:1423)
        at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:387)
        at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1312)
        at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1843)
        at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1808)
        at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:188)
Caused by: org.graalvm.polyglot.PolyglotException: java.net.ConnectException: Connection refused
        at <js>.WebSocket(websocket.js:132)
        at <js>.setupWS(insight.js:26914)
        at <js>.connect(insight.js:27190)
        at <js>.WebsocketProvider(insight.js:27099)
        at <js>.attachProvider(insight.js:27205)
        at <js>.poly_enso_eval(insight.js:27239)

no idea what's going on...

ConnectException

but the java.net.ConnectException is really raised.

JaroslavTulach avatar Apr 11 '25 14:04 JaroslavTulach

๐Ÿงช Storybook is successfully deployed!

๐Ÿ“Š Dashboard:

  • ๐Ÿ‘€ Review changes: https://www.chromatic.com/build?appId=673738989fc3e8430364197e&number=2861
  • ๐Ÿ‘จโ€๐ŸŽจ Preview storybook: https://673738989fc3e8430364197e-olcdndvjeq.chromatic.com/

github-actions[bot] avatar May 07 '25 13:05 github-actions[bot]

โœจ GUI Checks Results

Summary

  • ๐Ÿงน Prettier: โŒ Failed
  • ๐Ÿงฐ GUI Checks: โŒ Failed
  • ๐Ÿ“š Storybook: โœ… Deployed

โš ๏ธ Action Required: Please review the failed checks and fix them before merging.

See individual check results for more details.

โ„น๏ธ Chromatic Tests Skipped

Chromatic visual regression tests were skipped for this PR.

To run the tests and deploy Storybook, add the CI: Run Chromatic label to this PR.

Note: We skip tests by default to optimize CI costs. Only run them when your UI changes are ready for review.

๐Ÿ‘ฎ Lint GUI Results

Check Results

  • ๐Ÿง  Typecheck: โœ… Passed
  • ๐Ÿงน Lint: โŒ Failed
  • ๐Ÿงช Unit Tests: โœ… Passed

ESLint

3 Warning(s):

app/ydoc-server-polyglot/src/insight.ts line 79

  • Start Line: 79
  • End Line: 79
  • Message: 'metadataJson' is assigned a value but never used. Allowed unused vars must match /^_/u.
  • From: [@typescript-eslint/no-unused-vars]

app/ydoc-server-polyglot/src/insight.ts line 91

  • Start Line: 91
  • End Line: 91
  • Message: 'frame' is defined but never used. Allowed unused args must match /^_/u.
  • From: [@typescript-eslint/no-unused-vars]

app/ydoc-server-polyglot/src/insight.ts line 121

  • Start Line: 121

  • End Line: 121

  • Message: 'p' is assigned a value but never used. Allowed unused vars must match /^_/u.

  • From: [@typescript-eslint/no-unused-vars]

๐ŸŽญ Playwright Test Results

Summary

  • โณ Duration: 5m 25s
  • โœ… Passed: 343 tests
  • ๐Ÿ”ด Failed: 1 tests
  • โš ๏ธ Flaky: 0 tests

๐Ÿ“ฅ Download Detailed Report

โš ๏ธ Action Required: Please review the failed tests and fix them before merging.

github-actions[bot] avatar May 23 '25 17:05 github-actions[bot]