You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
transformResult is invoked from two sites. The one on the success path sits inside dispatch()'s try; the one on the thrown path sits inside the catch, with no try of its own. The same hook, given the same failing implementation, therefore behaves in two opposite ways — and the uncontained one is the common mutation shape.
Tested against next @ a536e29b, built from source, Node 24.19.
Reproduction
One transformResult that throws, two server functions — one returning, one doing throw redirect("/after"):
Sanitization is bypassed. The raw HOOK-BOOM message reaches the caller, where the contained path correctly redacted it to Internal Server Error. That inverts the production error-disclosure policy for exactly the path that escaped.
It is broader than one hook
Every hook reached from that catch escapes the same way. transformFlightResult, invoked via foldFlightData, has the identical split — swap the hook for { collectFlightData: () => ({k:1}), transformFlightResult: () => { throw new Error("FLIGHT-BOOM"); } } and add X-Single-Flight: true:
ret -> status 500
thr -> HANDLER REJECTED -> FLIGHT-BOOM
So the fix wants to contain the whole thrown-path branch, not just the one call. Wrapping only transformResult leaves transformFlightResult escaping through the same hole.
Why the asymmetry matters more than it looks
The thrown path is not an edge case — throw redirect(...) is the standard shape for a mutation that navigates, and throw is how a server function signals failure at all. So a result policy that an author tested against every returned value works fine until the first redirect, and then takes down the request, loses the session cookie the stub was carrying, and leaks its own error text.
It is also the shape an author cannot easily discover: the hook is one function, its docblock describes one contract, and nothing at the call site suggests the containment differs by path.
Compare the sibling containment twenty lines away in the flight fold: collectFlightData is wrapped in a per-source try/catch with a comment explaining precisely why a hook's failure must not cost a committed mutation its outcome. transformFlightResult, in the same function, is bare. The reasoning already exists in the tree; it is applied unevenly.
Options
Contain the whole thrown-path branch — wrap the catch's control-flow branch so its catch re-enters the existing sanitized-error tail. Verified: both paths answer 500 | Internal Server Error | Set-Cookie ["sid=keepme"], the flight-hook variant is contained too, and the suite is unchanged (48 files / 524 passed / 2 skipped). Wrapping only the transformResult call also passes but leaves transformFlightResult escaping.
Wrap it and attribute it — same containment, but tag the resulting error as originating in the hook rather than in the function, so an author is not told "my function failed" when their result policy failed. Addresses the related misattribution problem (see below) at the same seam.
Let it escape on both paths. Consistent in the other direction, and defensible if the position is that a result-policy failure is a programming error the host should see. But it would regress the return path's current behaviour, which is the safer one, and it would drop cookies on every hook failure.
Document the asymmetry. I would argue against it: the contract cannot be honoured by an author who does not know which path a given call will take.
(1) is the obvious minimum; (2) is worth considering while the code is open, because the misattribution is a real cost — a contained hook failure is currently byte-identical at the client to a genuinely failing server function (Error: Internal Server Error), and for wrapInvocation the function has not even run. That is a separate finding and I am happy to file it separately if you would rather keep this one narrow.
Happy to send a PR with the containment plus a regression test asserting both paths produce the same status, the same sanitized message, and the same surviving cookie, with a non-throwing hook as the control.
Summary
transformResultis invoked from two sites. The one on the success path sits insidedispatch()'stry; the one on the thrown path sits inside thecatch, with notryof its own. The same hook, given the same failing implementation, therefore behaves in two opposite ways — and the uncontained one is the common mutation shape.Tested against
next@a536e29b, built from source, Node 24.19.Reproduction
One
transformResultthat throws, two server functions — one returning, one doingthrow redirect("/after"):Three differences, all in the wrong direction on the thrown path:
Set-Cookieis lost. Refusals after createEvent drop the response stub's Set-Cookie silently #3159 made every exit fold the stub, but an escape is not an exit — it never reachescommitEventResponse.HOOK-BOOMmessage reaches the caller, where the contained path correctly redacted it toInternal Server Error. That inverts the production error-disclosure policy for exactly the path that escaped.It is broader than one hook
Every hook reached from that
catchescapes the same way.transformFlightResult, invoked viafoldFlightData, has the identical split — swap the hook for{ collectFlightData: () => ({k:1}), transformFlightResult: () => { throw new Error("FLIGHT-BOOM"); } }and addX-Single-Flight: true:So the fix wants to contain the whole thrown-path branch, not just the one call. Wrapping only
transformResultleavestransformFlightResultescaping through the same hole.Why the asymmetry matters more than it looks
The thrown path is not an edge case —
throw redirect(...)is the standard shape for a mutation that navigates, andthrowis how a server function signals failure at all. So a result policy that an author tested against every returned value works fine until the first redirect, and then takes down the request, loses the session cookie the stub was carrying, and leaks its own error text.It is also the shape an author cannot easily discover: the hook is one function, its docblock describes one contract, and nothing at the call site suggests the containment differs by path.
Compare the sibling containment twenty lines away in the flight fold:
collectFlightDatais wrapped in a per-sourcetry/catchwith a comment explaining precisely why a hook's failure must not cost a committed mutation its outcome.transformFlightResult, in the same function, is bare. The reasoning already exists in the tree; it is applied unevenly.Options
catch's control-flow branch so itscatchre-enters the existing sanitized-error tail. Verified: both paths answer500 | Internal Server Error | Set-Cookie ["sid=keepme"], the flight-hook variant is contained too, and the suite is unchanged (48 files / 524 passed / 2 skipped). Wrapping only thetransformResultcall also passes but leavestransformFlightResultescaping.(1) is the obvious minimum; (2) is worth considering while the code is open, because the misattribution is a real cost — a contained hook failure is currently byte-identical at the client to a genuinely failing server function (
Error: Internal Server Error), and forwrapInvocationthe function has not even run. That is a separate finding and I am happy to file it separately if you would rather keep this one narrow.Happy to send a PR with the containment plus a regression test asserting both paths produce the same status, the same sanitized message, and the same surviving cookie, with a non-throwing hook as the control.