Repository navigation
fix(router): end and log SSR failures after the response started in dev - #9293
Conversation
🦋 Changeset detectedLatest commit: 15be63e The changes in this PR will be included in the next version bump. This PR includes changesets to release 8 packages
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 |
|
Caution Review failedThe pull request is closed. No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Development SSR error completion packages/qwik-router/src/middleware/request-handler/user-response.ts, packages/qwik-router/src/middleware/request-handler/user-response.unit.ts, .changeset/dev-ssr-error-response.md |
When headers have been sent, runOnce enters error-response handling. In development, it logs the error and uses the original error in the response. In production, it uses “Internal Server Error.” Tests check output, writer closure, and development logging. The changeset declares a patch release. |
Priority:
Estimated code review effort: 2 (Simple) | ~8 minutes
Change: Bug fix · Severity of issue fixed: Medium
Possibly related issues
- [🐞] Production SSR render errors are never logged or passed to
next()(node middleware returns onheadersSentfirst) #9191: Both concern SSR errors after headers are sent. This PR handles the development response;#9191concerns production error logging and propagation. - [🐞] dev: an error thrown while rendering a component on the server, with no
<Catch>, leaves the response open forever and is not logged #9252: This issue describes the development SSR error that remains unlogged and leaves the response open.
Possibly related PRs
- QwikDev/qwik#9048: Both PRs handle uncaught SSR errors. This PR covers errors after headers are sent;
#9048covers middleware crashes before headers are sent. - QwikDev/qwik#9055: Both address failures during SSR. This PR finalizes errors that reach the request handler after the response starts;
#9055handles loader failures. - QwikDev/qwik#9050: Both involve request-handler error handling and
ServerError.
Suggested reviewers: maiieul
Merge Risk: 
f1a09
-
The development SSR error response can be merged after normal checks. The separate server-function stream issue remains outside this change.
Security architecture risk: 
f1a09
-
The inspected change preserves HTML escaping and generic production error messages. No new security vulnerability was established. Remaining uncertainty concerns completion when a response stream is already locked or fails during writing or closing.
Retained concerns
No architecture-level concerns identified.Security review details
-
Security Blast Radius
- observed — The new diagnostic payload and original-error logging are gated on development mode and a response that has already started. Production continues to receive a generic error message. The inspected evidence does not establish which users or networks can reach deployed development servers.
Security Findings and Attack Paths
- inferred — The routed error-message-to-HTML path does not establish an introduced injection vulnerability: the newly supplied development message still traverses the unchanged escaping formatter. Request influence over application error contents and deployment exposure were not demonstrated, so this conclusion is limited to the inspected path rather than a general safety guarantee.
Trust Boundaries and Controls
- observed — The relevant boundary is application error data becoming browser-visible HTML and server-side diagnostics. The formatter encodes ampersands and angle brackets for its text slots, while the production branch substitutes a generic message. Already-committed headers are not rewritten.
Resilience and Maintainability Implications
- inferred — The successful unlocked-stream path improves failure containment by ending responses that previously remained open in development. It does not provide a universal termination guarantee: adapter-level recovery after rejection and cleanup by an existing writer owner remain unverified. The added regression tests exercise the unlocked mocked-stream case.
-
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: ending and logging SSR failures after the response starts in development. Linked Issues check Issue #9252requires development SSR failures without a Catch boundary to be logged and to finish after the response starts.user-response.tsnow enters the error path whenrequestEv.headersSenti…Out of Scope Changes check The changes stay within issue #9252. They update the router error path, add focused regression tests, and add a router changeset for the fix. No unrelated behavior or files are changed.Full details: Linked Issues check
-
Explanation
Issue
#9252requires development SSR failures without a Catch boundary to be logged and to finish after the response starts.user-response.tsnow enters the error path whenrequestEv.headersSentis true, logs the error in development, writes the development error page with the original error, and closes the writer. The new unit test verifies the error text, logging, and writer close. The production test verifies the generic error page remains unchanged.
-
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
@qwik.dev/core
@qwik.dev/router
@qwik.dev/utils
eslint-plugin-qwik
create-qwik
@qwik.dev/optimizer
@qwik.dev/devtools
@qwik.dev/mcp
commit: |
Closes #9252
Closes #9244
In dev, when a component throws during SSR and there is no
<Catch>boundary, the request hangs forever and the error is never loggedCause
runNextonly finished the response on an error when!isDevIn dev the error is left to the adapter, but once the response has started (the shell was already flushed) the middleware bails out onheadersSentand nothing is written or loggedFix
When the response has already started, handle dev like production: log the error and write the 500 error page before closing the stream
Production behavior is unchanged
Verification
main(nothing written to the response) and passes with the fixpnpm vitest run packages/qwik-router— 845 passedpnpm tsc.checkeslintandprettier --checkon the changed filesSummary by CodeRabbit