Skip to content

fix(ssr): load node:http after start.instrument in the node entry - #422

Open
brenelz wants to merge 1 commit into
nextfrom
fix/418-node-entry-instrument-first
Open

brenelz wants to merge 1 commit into
nextfrom
fix/418-node-entry-instrument-first

Conversation

@brenelz

@brenelz brenelz commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

Fixes #418.

With start: { node: true, instrument }, dist/server/node.js loaded node:http before the instrument module ran, so OpenTelemetry's HTTP instrumentation never patched the server and no request got an HTTP server span.

The runtime inlined into node.js statically imports node:http (plus node:fs, node:stream and the rest). ESM links every static import before any module body runs. So node:http was loaded before ./server.js could await import(instrument), and the module hooks the instrument registers never saw it.

When instrument is set, node.js is now a wrapper with no static imports. It awaits ./server.js (which runs the instrument, then the handler graph) and then imports the unchanged runtime from a new sibling file, node-runtime.js. Without instrument, node.js stays the same single self-contained file, apart from one new config field. The runtime's "listen when run as the program" check now compares argv[1] against SOLID_NODE_CONFIG.entry (node.js) instead of its own file name. So node dist/server/node.js still listens, and running node-runtime.js directly does not start a server that skips the instrument. The README notes the extra file.

Verification:

  • node test/run.mjs node in examples/start-ssr passes 89/89. The new sub-run builds with START_NODE=1 SSR_INSTRUMENT=1. It asserts that node.js has no static imports, that it listens when run, that the instrument saw node:http unloaded (process.moduleLoadList), and that importing it exports createListener, listener and serve without listening. Against origin/next, two of those assertions fail (httpLoaded: true).
  • node test/run.mjs middleware passes 99/99.
  • With a real NodeSDK and @opentelemetry/instrumentation-http loaded from the instrument chunk, node dist/server/node.js recorded no spans before the fix and one SERVER span per request after it.

Open question: node.js now has two layouts depending on instrument. A single layout would also work: always emit the wrapper, or make the runtime load node:* lazily. I kept the change scoped to the option that needs it.

🤖 Generated with Claude Code

@changeset-bot

changeset-bot Bot commented Oct 16, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: f3fb6f2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@solidjs/vite-plugin Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

pkg-pr-new Bot commented Oct 16, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/@solidjs/vite-plugin@422

commit: f3fb6f2

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant