Implement websocket API#83
Conversation
This comment has been minimized.
This comment has been minimized.
|
Few general remarks:
|
…ored node.js tests for ws
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
- Use Object.defineProperties with lazy getters on the default export to defer globalThis.WebSocket access until after WIRE_JS initialization. - Avoids both top-level await (which broke _requireEsm fallback path) and static import (which caused stack overflow during Wizer pre-init).
|
Failing CI tests fails in main branch too. Not sure how it was successful before. All 248 runtime tests is success in local by the way. |
This comment has been minimized.
This comment has been minimized.
|
|
||
| # Golem | ||
| golem-rust = { version = "1.7", default-features = false, optional = true } | ||
| golem-websocket = { version = "0.0.1" } |
There was a problem hiding this comment.
This should be feature-gated by golem (and actually we will need to change the npm script in golem to use not just full, but full,golem when compiling the base image)
There was a problem hiding this comment.
Ok, yes, I can change that
There was a problem hiding this comment.
Ok, I tried that, and it doesn't work for a variety of reasons...
Steps I followed
-
Made golem-websocket under golem feature flag: b238c5e
-
Then, installed this wasm-rquickjs and tested with golem after changing the npm script with features
fullandgolem
i.e, "compile-agent-template": "cargo component build --manifest-path=agent-template/Cargo.toml --release --features full,golem"
And then building it pnpm run build && pnpm run build-agent-template resulted in
error: failed to run custom build command for `libsqlite3-sys v0.36.0 (https://github.com/golemcloud/rusqlite?branch=v0.38.0-patched#40024c3f)`
- So just to remove sqlite from picture I also tested with just golem and not full
"compile-agent-template": "cargo component build --manifest-path=agent-template/Cargo.toml --release --features golem"`
and that resulted in
> cargo component build --manifest-path=agent-template/Cargo.toml --release --features golem
Generating bindings for agent-guest (agent-template/src/bindings.rs)
Updating crates.io index
Locking 1 package to latest Rust 1.94.0 compatible version
Adding golem-websocket v0.0.1
Compiling agent-guest v0.0.1 (/Users/afsalthaj/projects/ribbb/golem/sdks/ts/packages/golem-ts-sdk/agent-template)
Building [=======================> ] 364/365: agent-guest
Finished `release` profile [optimized] target(s) in 33.17s
Creating component agent-template/target/wasm32-wasip1/release/agent_guest.wasm
error: failed to upgrade `golem:api/host@1.3.0` to `golem:api/host@1.5.0`, was this semver-compatible update not semver compatible?
Caused by:
0: failed to merge interfaces
1: expected type `account-id` to be present
ELIFECYCLE Command failed with exit code 1.
/Users/afsalthaj/projects/ribbb/golem/sdks/ts/packages/golem-ts-sdk:
ERR_PNPM_RECURSIVE_RUN_FIRST_FAIL @golemcloud/golem-ts-sdk@0.0.0 build-agent-template: `pnpm run generate-agent-template && pnpm run compile-agent-template && pnpm run copy-agent-template`
Exit status 1
ELIFECYCLE Command failed with exit code 1.So for now I am reverting the feature gate commit and wait for your feedback - just to make sure whatever actively in this PR works with golem PR and succeeds ts integration tests
There was a problem hiding this comment.
- you can't remove sqlite, we are depending on it in the sdk. building the base image requires wasi_sdk now, check the CI scripts how they install it.
- the wit interface errors are suggesting that something is very mixed up for you, having old pre-1.5 wit packages
| } | ||
|
|
||
| mod webstreams; | ||
| mod websocket; |
There was a problem hiding this comment.
should be feature gated with golem
There was a problem hiding this comment.
I see. I actually tried it, but golemcloud/golem#3076 was failing with "Websocket not found". I guess the generated crate during base-wasm generation in Golem has only "normal" features included. I will try and understand a bit more soon
| }, | ||
| "parallel/test-webcrypto-wrap-unwrap.js": { "skip": true, "reason": "requires CFRG key (Ed448/X25519/X448) DER export support" }, | ||
| "parallel/test-websocket.js": { "skip": true, "reason": "newly discovered, not yet evaluated" }, | ||
| "parallel/test-websocket.js": {}, |
There was a problem hiding this comment.
How is this passing if we don't have an actual websocket host implementation in the test runnet? :)
There was a problem hiding this comment.
It is only testing very minimal stuff..
https://github.com/nodejs/node/blob/v22.14.0/test/parallel/test-websocket.js
nothing much in that test
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Amp-Thread-ID: https://ampcode.com/threads/T-019d4464-623a-77a8-a2bf-1daaca6c4822 Co-authored-by: Amp <amp@ampcode.com>
Fixes part of golemcloud/golem#3054
Tested this change in real here: golemcloud/golem#3076.
This needs golem-rust with websocket changes published