Skip to content

Conversation

4e6
Copy link
Contributor

@4e6 4e6 commented Jan 2, 2025

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 4e6 self-assigned this Jan 2, 2025
@4e6
Copy link
Contributor Author

4e6 commented Jan 2, 2025

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

Copy link
Member

@JaroslavTulach JaroslavTulach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Dmitry for finding a way to convert insight offsets to ASTs!

const result = ctx.returnValue(frame)
if (result && spanMap) {
const key = Ast.nodeKey(ctx.charIndex, ctx.charEndIndex - ctx.charIndex)
const asts = spanMap.nodes.get(key)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Whenever we get an insight.on('return', ...) event, we look appropriate asts and update them. I am not sure how fast this is, but it certainly seems good enough for demonstration purposes.

}
}, {
expressions: true,
statements: false,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe n = 42 would be a statement - e.g. if you want to observe when "IDE nodes" are being assigned to, you may want to observe statements.

@JaroslavTulach
Copy link
Member

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?


const d = new Y.Doc()
const project = new DistributedProject(d)
//const provider = attachProvider('ws://[::1]:5976/project', 'index', d)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uncomment attachProvider method to reproduce the issue

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I comment out the line with 814eb91 I get following errors:

Initializing Insight: {"id":"insight","version":"1.2"}
attachProvider ws://[::1]:5976/project index
[WARN] [2025-03-14T09:43:00.619] [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:26888)
        at <js>.connect(insight.js:27164)
        at <js>.WebsocketProvider(insight.js:27073)
        at <js>.attachProvider(insight.js:27179)
        at <js>.poly_enso_eval(insight.js:27213)

I hope that means the WebSocket infrastructure is working fine, it is just misconfigured. What do you think, @4e6?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@4e6
Copy link
Contributor Author

4e6 commented Mar 10, 2025

@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')

@JaroslavTulach
Copy link
Member

JaroslavTulach commented Mar 13, 2025

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
Copy link
Member

JaroslavTulach commented Mar 13, 2025

@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:

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

Update on Apr 11, 2025:

@JaroslavTulach
Copy link
Member

JaroslavTulach commented Mar 14, 2025

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

@JaroslavTulach JaroslavTulach force-pushed the wip/db/11477-execute-yjs-insight branch from e8558ca to f383682 Compare May 12, 2025 09:35
@github-actions
Copy link

github-actions bot commented May 23, 2025

✨ 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.

#length;

constructor(buffer) {
constructor(buffer, offset, length) {
Copy link
Member

@JaroslavTulach JaroslavTulach May 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is here to support Buffer.from(arrayBuffer[, byteOffset[, length]]). Originally I tried to just extract offset and length from buffer, create a copy of such buffer and pass it into the old constructor. But that doesn't seem to work.

The Buffer.from(arrayBuffer[, byteOffset[, length]]) factory method is supposed to provide live view of the buffer. E.g. further changes are still propagated to the buffer.

As such we are extracting the slice lazily in buffer() getter. With that approach we pass thru the new tests in 8e21525

@JaroslavTulach JaroslavTulach linked an issue May 24, 2025 that may be closed by this pull request
*
* The {@code enso-engine-*?/enso-*?/component/} represents path to the @{code component} directory
* which needs to be adjusted to proper path on each operating system. All lines may need to be
* concatenated into a single line.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The WebSocketClient should be a useful utility for testing compatibility of the emulation with real node.js implementation.

System.out.printf("Script %s evaluated to %s\n", code, res);
var console = System.console();
for (int i = 0; i < 10; i++) {
var line = console.readLine("js> ");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This console.readLine seems to work surprisingly well. It supports arrow movements and even provides history of previous entries. Isn't it time to replace JLine with it? Isn't the console actually backed by JLine (repackaged)?

var keyAndValue = args.get(atEnv + 1).split("=", 2)
if (jvmOptName == keyAndValue(0)) {
prevValue = prevValue + " " + keyAndValue(1)
val (key, value) = {
Copy link
Member

@JaroslavTulach JaroslavTulach May 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With these changes, it is possible to:

$ 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

e.g. to influence the environment variable ENSO_JVM_OPTS from sbt shell itself (please concatenate the above example to single sbt command line). No need to restart sbt to change the arguments anymore.

if (asts) {
const resultValue = resultToString(result)
for (const ast of asts) {
const editAst = syncModule.getVersion(ast)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deliver expression updates via Ydoc AST elements Allow to execute Y.js and Insight together

3 participants