fix: SSR error prod-silence keys on the dev flag, not NODE_ENV#484
Conversation
vivek7405
left a comment
There was a problem hiding this comment.
Went through the threading end to end. The fix is right and the back-fill of ctx.dev is safe (request-scoped ctx, only set when undefined). Two things on the record. First, my original integration test passed for the wrong reason: under createRequestHandler the render effectively sees NODE_ENV=production, so reverting the dev-gating did not make the PROD case leak, only the DEV case flipped. The real counterfactual lives at the renderToString({suspenseCtx:{dev}}) layer with NODE_ENV deleted, which I added. Second, processSuspenseElements was not getting the dev flag, so a throw inside an inline boundary on a ctx-absent render fell back to NODE_ENV (masked by a double-render). Both fixed.
vivek7405
left a comment
There was a problem hiding this comment.
Second look at the threading completeness. Caught that my earlier processSuspenseElements fix never actually landed: a counterfactual git checkout reverted it before I committed, so the helper was still re-deriving ctx && ctx.dev instead of taking the dev param. That left a throwing component inside an inline (ctx-absent) falling back to NODE_ENV (masked only by the outer walk re-rendering it, which also wastes a render). Re-applied it and unified BOTH suspense helpers (processSuspenseElements + streamSuspenseBoundaries) onto the threaded dev param, so they can no longer diverge from the walk.
vivek7405
left a comment
There was a problem hiding this comment.
Last pass: the dev threading is complete across all four error paths (the walk, an inline non-streaming boundary, a streamed boundary, and a rejected streamed boundary), and the only remaining ctx.dev reads are the legitimate back-fills. Two JSDoc blocks (injectDSD + streamSuspenseBoundaries) had not documented the new dev param; filled those in.
vivek7405
left a comment
There was a problem hiding this comment.
Final pass is clean. The dev flag threads through every SSR error path now (the walk, inline and streamed boundaries, and a rejected streamed boundary), the back-fill is the only ctx.dev read and is request-scoped, and the core-layer counterfactual genuinely fails if the gating is removed. Ready to merge.
defaultSSRErrorTemplate (the per-component SSR error isolation) and the renderToStream boundary catch gated prod-silence on NODE_ENV. But webjs keys prod on the CLI dev flag, and webjs start does not export NODE_ENV=production, so a bare prod launch leaked a component error message to the client. Thread the server dev flag through the SSR render context (ssr.js sets it on the suspenseCtx; renderToString/renderToStream back-fill and thread it to injectDSD and the boundary catch). Prod-silence now triggers whenever dev is false. A context-free renderToString with no dev signal falls back to NODE_ENV, preserving the prior behaviour for that path. The component-isolation path, the page-streaming boundary, and the core renderToStream boundary now agree on the same signal.
loadingTemplates(route, ctx, dev) has a dev parameter, not opts. The prod-silence dev threading wrongly referenced opts.dev there, throwing a ReferenceError that the per-file catch swallowed, dropping the wj-loading template. Use the dev param.
…actual
processSuspenseElements did not receive the dev flag, so a throwing
component inside an inline (ctx-absent) <webjs-suspense> reached
defaultSSRErrorTemplate with dev=undefined, falling back to NODE_ENV (a
latent leak under bare webjs start, masked only by a redundant
double-render). Thread dev through it.
Add a core-layer renderToString({suspenseCtx:{dev}}) test with NODE_ENV
unset: dev:false stays silent (a genuine leak counterfactual that fails
when the dev-gating is reverted), dev:true surfaces. The createRequestHandler
NODE_ENV is effectively production during a render, so the integration
tests alone could not prove the prod-leak prevention.
Re-apply (and complete) the dev threading: processSuspenseElements and streamSuspenseBoundaries now take dev and use it for injectDSD and the boundary defaultSSRErrorTemplate, instead of re-deriving ctx && ctx.dev. This silences a throwing component inside an inline (ctx-absent) <webjs-suspense> in prod (the ctx.dev derivation was undefined there, so it fell back to NODE_ENV), and removes the walk-dev-vs-ctx.dev divergence the suspense helpers had.
bdb3a32 to
c33da2d
Compare
Summary
Closes #483.
defaultSSRErrorTemplate(the per-component SSR error isolation) and the corerenderToStreamboundary catch gated prod-silence onNODE_ENV. But webjs keys prod on the CLIdevflag (the deployment docs say so), andwebjs startdoes NOT exportNODE_ENV=production, so a bare prod launch leaked a thrown component's error message (a DB error, a stack-derived path) to the client. This also made #482's "the two streaming error paths now agree" only true whenNODE_ENV=productionwas set.The fix threads the server
devflag through the SSR render context:ssr.jsstamps it on the suspenseCtx;renderToString/renderToStreamsource it (opts.dev wins, else the ctx) and back-fill the ctx so a streamed sub-render inherits it, then thread it toinjectDSDand the boundary catch. Prod-silence now triggers wheneverdevis false, regardless ofNODE_ENV. A context-freerenderToStringwith no dev signal falls back toNODE_ENV, preserving the prior behaviour for a bare unit/embedded render. The component-isolation path, the page-streaming boundary, and the corerenderToStreamboundary now agree on the same signal.Test plan
packages/server/test/dev/ssr-error-prod-silence.test.js): a component whoserender()throws, driven throughcreateRequestHandlerwithNODE_ENVunset. PROD (dev:false) emits NO message (still isolates, siblings render); DEV (dev:true) surfaces it via the isolation template (data-webjs-error); a context-freerenderToStringfalls back to NODE_ENV. Counterfactual verified: neutering the dev-gating makes the PROD case leak.test/ssr493 pass (caught and fixed a regression whereloadingTemplates's localdevparam was wrongly referenced asopts.dev).Definition of done
error-handling,components.md) say "dev"/"prod", not "NODE_ENV", so they were aspirationally correct; this fix makes the "silent in prod" claim actually true underwebjs start. The deployment doc already documents that webjs keys on thedevflag.@webjsdev/coredist is gitignored and CI rebuilds it from src; I rebuilt it locally so the createRequestHandler path exercises the fix.)