Skip to content

src: add a flag to keep the embedder's wasm streaming callback - #65690

Open
codebytere wants to merge 1 commit into
nodejs:mainfrom
codebytere:isolate-settings-wasm-streaming
Open

src: add a flag to keep the embedder's wasm streaming callback#65690
codebytere wants to merge 1 commit into
nodejs:mainfrom
codebytere:isolate-settings-wasm-streaming

Conversation

@codebytere

Copy link
Copy Markdown
Member

SetIsolateMiscHandlers() always installs Node.js's WebAssembly.compileStreaming() handler, which goes through the Environment's fetch-based implementation. An embedder that supplies its own streaming callback has to put it back after every NewIsolate() / SetIsolateUpForNode() call.

This adds SHOULD_NOT_SET_WASM_STREAMING_CALLBACK to IsolateSettingsFlags, following SHOULD_NOT_SET_PROMISE_REJECTION_CALLBACK and SHOULD_NOT_SET_PREPARE_STACK_TRACE_CALLBACK (#36447), so the embedder's callback is left alone when the flag is set. Default behavior is unchanged.

Tests: new cctest EnvironmentTest.KeepsEmbedderWasmStreamingCallbackWhenAsked installs a callback, calls SetIsolateUpForNode() with and without the flag, and checks which one WebAssembly.compileStreaming() reaches; it fails on main and passes here.

Refs: #36447


Disclosure: the code, test and this description were written by Claude Code, directed and reviewed by @codebytere.

`SetIsolateMiscHandlers()` always installs Node.js's
`WebAssembly.compileStreaming()` implementation, which is backed by the
Environment's fetch-based handler. An embedder that provides its own
streaming callback (for example one wired to its own network stack) has
to re-install it after every `SetIsolateUpForNode()` or `NewIsolate()`
call. Add `SHOULD_NOT_SET_WASM_STREAMING_CALLBACK` next to the existing
`SHOULD_NOT_SET_PROMISE_REJECTION_CALLBACK` and
`SHOULD_NOT_SET_PREPARE_STACK_TRACE_CALLBACK` flags so it can opt out
the same way.

Refs: nodejs#36447
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Sep 1, 2026
@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.04%. Comparing base (705646f) to head (c34940c).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #65690      +/-   ##
==========================================
- Coverage   90.07%   90.04%   -0.03%     
==========================================
  Files         754      754              
  Lines      256395   256397       +2     
  Branches    48499    48494       -5     
==========================================
- Hits       230937   230883      -54     
- Misses      16569    16630      +61     
+ Partials     8889     8884       -5     
Files with missing lines Coverage Δ
src/api/environment.cc 78.93% <100.00%> (+0.53%) ⬆️
src/node.h 92.45% <ø> (ø)

... and 32 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run.

2 participants