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
{{ message }}
Repository navigation
fix(router): reject server$ calls that get a non-server$ response - #9378
When a server$ request was answered with an HTML page (for example a middleware redirecting ?qfunc= requests to / on an expired session), the call resolved undefined. The client compared the whole Content-Type header, so text/html; charset=utf-8 matched none of the branches and the function just fell off the end. The same happened for application/json; charset=utf-8 and text/plain; charset=utf-8, which is what json() and text() in a request handler send.
Fix
Moved the response handling out of serverQrl into readServerFunctionResponse so it can be unit tested (in the vitest setup isServer is replaced at compile time, so I couldn't drive the client branch through server$ itself).
Compare only the media type (lowercased, parameters dropped).
Dropped text/html from the string branch. It's not in the Accept header the client sends, so it's never a server$ payload. Anything that doesn't match now throws an error with the status and content type, so the promise rejects.
Testing
pnpm vitest run packages/qwik-router/src/runtime/src/server-functions.unit.ts
New tests cover an HTML page (with and without charset) rejecting, and json/text with ; charset=utf-8 still resolving or throwing on 4xx. Both fail on main and pass with this change.
All qwik-router unit tests pass (885). The only failures in the full test.unit run here are the qwik-vite optimizer tests, which need the native optimizer binding I do not have built locally.
Summary by CodeRabbit
Bug Fixes
server$ calls now reject unexpected responses, such as HTML pages, instead of resolving to undefined.
Response content types are recognized consistently, including when they use different capitalization or include parameters.
Error responses preserve their response body as the rejection value. JSON and plain-text responses continue to be handled according to their status.
server$ compared the whole Content-Type header, so 'text/html; charset=utf-8'
(e.g. a page after a middleware redirect) matched no branch and the call
resolved undefined. Compare only the media type and throw for anything
that is not a server$ payload.
FixesQwikDev#9335
No actionable comments were generated in the recent review.
Walkthrough
serverQrl delegates client response handling to readServerFunctionResponse. The helper parses supported response types and rejects unsupported types, including HTML. Unit tests cover media type parameters, case-insensitive matching, and error responses.
Changes: Server function response handling
Layer / File(s)
Summary
Response parsing and validation packages/qwik-router/src/runtime/src/server-functions.ts, packages/qwik-router/src/runtime/src/server-functions.unit.ts, .changeset/server-fn-html-response.md
serverQrl delegates response handling to readServerFunctionResponse. The helper parses supported content types without parameters and matches them case-insensitively. Unsupported types, including HTML, produce an unexpected-response error. Tests cover these cases and status-based errors. The changeset records the fix.
The change rejects unsupported responses while preserving supported JSON and text handling. No actionable merge-blocking risk is established; merge after normal checks pass.
Pre-merge checks: 5
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title names the main change: server$ calls now reject when they receive a non-server$ response, such as an HTML page. The changeset, the new readServerFunctionResponse helper, and its tests all ma…
Linked Issues check
[#9335] Requirement: a server$ call must reject when the response is an HTML page, not resolve undefined. At head, serverQrl returns readServerFunctionResponse(res, abortSignal) (server-functions.ts).…
Out of Scope Changes check
All changes relate to #9335. The server-functions.ts diff moves the existing response-handling logic into readServerFunctionResponse. The qwik-json-stream, qwik-json, and application/json branches kee…
Docstring Coverage
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Full details: Title check
Explanation
The title names the main change: server$ calls now reject when they receive a non-server$ response, such as an HTML page. The changeset, the new readServerFunctionResponse helper, and its tests all match this change. The title follows the conventional commit format and is specific enough for a teammate scanning history.
Full details: Linked Issues check
Explanation
[#9335] Requirement: a server$ call must reject when the response is an HTML page, not resolve undefined. At head, serverQrl returns readServerFunctionResponse(res, abortSignal) (server-functions.ts). That helper strips parameters from Content-Type (split on ';', trim, lowercase) before matching. The text/html branch was removed. Any unmatched media type, including text/html with or without charset, now throws Error('server$ got an unexpected response: <status> <content-type>'). The returned promise therefore rejects, as the issue expects. Charset-bearing application/json and text/plain responses still resolve or throw as before. The unit tests in server-functions.unit.ts cover the HTML rejection, charset handling, a 401 text/plain rejection value, and the Application/JSON casing. The tests call readServerFunctionResponse directly, not through server$ end to end. This is adequate because serverQrl returns that promise unchanged. I did not run the tests. The result relies on the reported test run and on reading the code. The client Accept header at head is 'application/json, application/qwik-json, text/qwik-json-stream, text/plain', so removing text/html from the string branch does not drop a payload type that the client requests.
Full details: Out of Scope Changes check
Explanation
All changes relate to #9335. The server-functions.ts diff moves the existing response-handling logic into readServerFunctionResponse. The qwik-json-stream, qwik-json, and application/json branches keep their original logic. The only behavior changes are the media-type normalization, the removal of text/html from the string branch, and the new error for unmatched types. These changes implement the issue. The unit test diff tests the new helper. The changeset file adds a patch note for @qwik.dev/router describing the fix. The helper export is marked @internal and exists only for tests. No unrelated code changes appear in the reviewed diff.
Full details: Docstring Coverage
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 2 files. (1 skipped: 1 unsupported.)
Finishing Touches
Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When a
server$request was answered with an HTML page (for example a middleware redirecting?qfunc=requests to/on an expired session), the call resolvedundefined. The client compared the wholeContent-Typeheader, sotext/html; charset=utf-8matched none of the branches and the function just fell off the end. The same happened forapplication/json; charset=utf-8andtext/plain; charset=utf-8, which is whatjson()andtext()in a request handler send.Fix
serverQrlintoreadServerFunctionResponseso it can be unit tested (in the vitest setupisServeris replaced at compile time, so I couldn't drive the client branch throughserver$itself).text/htmlfrom the string branch. It's not in theAcceptheader the client sends, so it's never aserver$payload. Anything that doesn't match now throws an error with the status and content type, so the promise rejects.Testing
pnpm vitest run packages/qwik-router/src/runtime/src/server-functions.unit.ts; charset=utf-8still resolving or throwing on 4xx. Both fail on main and pass with this change.Fixes #9335
All qwik-router unit tests pass (885). The only failures in the full
test.unitrun here are the qwik-vite optimizer tests, which need the native optimizer binding I do not have built locally.Summary by CodeRabbit
server$calls now reject unexpected responses, such as HTML pages, instead of resolving toundefined.