Skip to content

fix(start-client-core): reject untagged non-JSON server function responses - #8443

Open
mixelburg wants to merge 1 commit into
TanStack:mainfrom
mixelburg:fix/untagged-server-fn-response
Open

mixelburg wants to merge 1 commit into
TanStack:mainfrom
mixelburg:fix/untagged-server-fn-response

Conversation

@mixelburg

@mixelburg mixelburg commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

🎯 Changes

Fixes #8333.

When something between the browser and the server answers /_serverFn/* itself (reverse proxy, bot challenge, captive portal), the response carries no x-tss-* header at all. If that response was 2xx and non-JSON, getResponse returned the Response object as the server function's result — the caller silently received a challenge page where it expected data, and the failure only showed up later (data.map is not a function).

An untagged response did not come from the server-functions handler, so it now rejects with its body, which is what the same function already does for the untagged non-2xx case. Deliberate raw responses keep working: the server marks them with x-tss-raw and they return before this branch.

Added packages/start-client-core/tests/server-fn-fetcher.test.ts covering the untagged 200 interstitial, the untagged error response, and the tagged raw response.

✅ Checklist

  • I have followed the steps in the Contributing guide.
  • I have tested code changes locally with the relevant test commands, or tests do not apply to this pull request.
  • I fully understand the code in this pull request, including any code generated with AI assistance.

🚀 Release Impact

  • This change affects published code, and I have generated a changeset.
  • This change is docs/CI/dev-only (no release).

Summary by CodeRabbit

  • Bug Fixes

    • Server function calls now report untagged, non-JSON responses as errors instead of returning them as successful results.
    • Error details from these responses are surfaced using the response body, improving visibility for unexpected proxy, challenge, or portal responses.
    • Responses explicitly marked for raw handling continue to be returned unchanged.
  • Tests

    • Added coverage for successful and error responses, including raw response handling.

@coderabbitai

coderabbitai Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: e1902e9b-2a0b-497e-a3a4-26ba85361c96

📥 Commits

Reviewing files that changed from the base of the PR and between a0b2ad9 and 910941b.

📒 Files selected for processing (3)
  • .changeset/untagged-server-fn-response.md
  • packages/start-client-core/src/client-rpc/serverFnFetcher.ts
  • packages/start-client-core/tests/server-fn-fetcher.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The client now rejects untagged, non-JSON server function responses by throwing their body text. Tests cover 200 and 403 responses and preserve raw Response handling. A patch changeset documents the behavior.

Changes

Server function response handling

Layer / File(s) Summary
Reject untagged non-JSON responses
packages/start-client-core/src/client-rpc/serverFnFetcher.ts, packages/start-client-core/tests/server-fn-fetcher.test.ts, .changeset/untagged-server-fn-response.md
getResponse now throws the body text for every untagged, non-JSON response. Tests cover 200 and 403 responses and verify that x-tss-raw responses still return a raw Response. The changeset records a patch release.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 91094

The intended handling for intermediary non-JSON responses is implemented with coverage for the primary response paths. No actionable merge risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.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. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: rejecting untagged, non-JSON server-function responses.
Description check ✅ Passed The description explains the problem, the behavior change, preserved raw-response behavior, tests added, issue reference, and release impact. All template sections are present and the checklist is com…
Linked Issues check ✅ Passed The change satisfies #8333. getResponse now throws the body for every untagged, non-JSON response, including successful 2xx responses. The new test covers a 200 text/html interstitial and a 403 te…
Out of Scope Changes check ✅ Passed The changes are within #8333. The implementation changes the affected fallback, the tests cover the reported and preserved behaviors, and the changeset documents the patch. No unrelated product behavi…
Full details: Docstring Coverage

Explanation

Docstring coverage is 50.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. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

Start: untagged 2xx non-JSON server-function response resolves as a raw Response instead of rejecting

1 participant