Skip to content

Comments

fix: set statusCode to 500 if error occurs#5541

Merged
schiller-manuel merged 1 commit intomainfrom
fix-5535
Oct 19, 2025
Merged

fix: set statusCode to 500 if error occurs#5541
schiller-manuel merged 1 commit intomainfrom
fix-5535

Conversation

@schiller-manuel
Copy link
Contributor

@schiller-manuel schiller-manuel commented Oct 19, 2025

fixes #5535

Summary by CodeRabbit

  • New Features

    • notFound is now available as a public API export from router core.
  • Bug Fixes

    • Improved status code computation to conditionally update based on not-found and error matches (404 or 500 respectively).
  • Tests

    • Added comprehensive test coverage for status code behavior when loaders throw exceptions under synchronous and asynchronous conditions.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 19, 2025

Walkthrough

The PR fixes SSR status code handling by computing appropriate HTTP status codes (404 for not-found, 500 for errors) when loaders throw exceptions. It exports the notFound function from core and adds comprehensive tests verifying correct status code behavior across synchronous and asynchronous scenarios.

Changes

Cohort / File(s) Summary
Core Router Logic
packages/router-core/src/router.ts
Adds conditional status code computation based on not-found or error matches; updates router state's statusCode to 404 or 500 respectively, replacing previous unconditional logic
Public API Export
packages/router-core/src/...
Exports notFound function as a public entity from @tanstack/router-core
Test Coverage
packages/react-router/tests/router.test.tsx
Renames test suite; updates root route creation pattern; adds comprehensive test cases verifying statusCode behavior when loader/beforeLoad throw errors or notFound, covering sync/async conditions and component rendering

Sequence Diagram

sequenceDiagram
    participant Client
    participant Router as Router (SSR)
    participant Loader
    participant StateManager as Router State
    
    Client->>Router: Navigate / Initial Load
    Router->>Loader: Execute loader/beforeLoad
    
    alt Loader throws Error
        Loader-->>Router: Error
        Router->>Router: Compute newStatusCode = 500
        Router->>StateManager: Update statusCode to 500
        Router-->>Client: Response (500)
    else Loader throws notFound
        Loader-->>Router: notFound()
        Router->>Router: Compute newStatusCode = 404
        Router->>StateManager: Update statusCode to 404
        Router-->>Client: Response (404)
    else Loader succeeds
        Loader-->>Router: Success
        Router->>Router: No status code change
        Router-->>Client: Response (200)
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

The changes involve conditional logic modifications in the router's load handling and new test coverage across multiple scenarios. The logic shift from unconditional to conditional status code assignment requires careful verification of edge cases, but the overall scope remains focused on status code handling.

Possibly related PRs

Poem

🐰 A rabbit hops through error streams,
Finding 404s in loader dreams,
No more 200 when things go wrong,
Proper status codes now belong,
SSR speaks the truth at last!

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "fix: set statusCode to 500 if error occurs" directly addresses the primary objective outlined in issue #5535, which requires that the HTTP status code be set to 500 when an error is thrown during server-side rendering. The title is specific and clearly communicates the main change—that the router now properly sets the status code to 500 for errors instead of leaving it at 200. While the changes also handle notFound scenarios with 404 status codes, the title appropriately focuses on the primary issue of error status code handling, which is acceptable since titles need not cover every detail of the changeset.
Linked Issues Check ✅ Passed The code changes directly address the requirements in issue #5535. The router.ts modification now computes a newStatusCode based on error or notFound matches, setting it to 500 for errors and 404 for notFound cases, which resolves the core problem where SSR status code remained 200 even when a loader threw an Error. The new tests comprehensively verify statusCode behavior when loaders/beforeLoad throw errors and notFound exceptions, confirming the fix works across synchronous and asynchronous scenarios. The export of notFound from @tanstack/router-core is necessary to support proper testing of these status code behaviors.
Out of Scope Changes Check ✅ Passed All code changes are directly scoped to addressing issue #5535. The modifications to router.ts implement the core fix by computing statusCode based on error and notFound matches. The export of notFound is a necessary supporting change to enable proper testing of notFound scenarios alongside error handling. The test changes, including root route creation pattern adjustments and new test cases, are all directly related to verifying the statusCode behavior when loaders throw errors or notFound exceptions, which is central to validating the fix.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-5535

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e87bd30 and 3d15cc7.

📒 Files selected for processing (2)
  • packages/react-router/tests/router.test.tsx (3 hunks)
  • packages/router-core/src/router.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use TypeScript in strict mode with extensive type safety across the codebase

Files:

  • packages/router-core/src/router.ts
  • packages/react-router/tests/router.test.tsx
packages/router-core/**

📄 CodeRabbit inference engine (AGENTS.md)

Keep framework-agnostic core router logic in packages/router-core/

Files:

  • packages/router-core/src/router.ts
packages/{react-router,solid-router}/**

📄 CodeRabbit inference engine (AGENTS.md)

Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/

Files:

  • packages/react-router/tests/router.test.tsx
🧠 Learnings (1)
📚 Learning: 2025-10-08T08:11:47.088Z
Learnt from: nlynzaad
PR: TanStack/router#5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.

Applied to files:

  • packages/react-router/tests/router.test.tsx
🧬 Code graph analysis (1)
packages/react-router/tests/router.test.tsx (3)
packages/react-router/src/index.tsx (6)
  • createMemoryHistory (118-118)
  • createRootRoute (258-258)
  • notFound (315-315)
  • createRoute (255-255)
  • createRouter (273-273)
  • RouterProvider (283-283)
packages/history/src/index.ts (1)
  • createMemoryHistory (568-614)
packages/router-core/src/index.ts (1)
  • notFound (377-377)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Test
  • GitHub Check: Preview
🔇 Additional comments (4)
packages/router-core/src/router.ts (1)

2138-2149: LGTM! Proper HTTP status code handling for SSR.

The logic correctly computes the status code after all matches are loaded:

  • 404 for not-found scenarios
  • 500 for error scenarios
  • Preserves existing status (typically 200) otherwise

This complements the earlier statusCode setting in the catch block (lines 2104-2114), which handles thrown exceptions, while this post-load check handles cases where matches have error/notFound status without throwing exceptions.

packages/react-router/tests/router.test.tsx (3)

11-11: LGTM! Correct import for test coverage.

The notFound import is necessary for the new statusCode tests that verify proper handling of not-found scenarios.


1936-1936: LGTM! Cleaner test setup.

Removing the root route component is appropriate here since the test focuses on statusCode behavior and child routes define their own components with the necessary test IDs.


1970-2136: LGTM! Comprehensive test coverage for statusCode behavior.

The test suite excellently validates the fix with:

  • DRY approach: Uses describe.each to test both synchronous and asynchronous scenarios
  • Complete coverage: Tests all four combinations (loader/beforeLoad × notFound/Error)
  • Proper assertions: Verifies statusCode transitions, correct error/notFound component rendering, and that route components are NOT rendered when errors occur
  • Clean setup: Appropriate use of act(), memory history, and testIds

The throwingFun helper elegantly handles the sync/async variants without code duplication.


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

@nx-cloud
Copy link

nx-cloud bot commented Oct 19, 2025

View your CI Pipeline Execution ↗ for commit 3d15cc7

Command Status Duration Result
nx affected --targets=test:eslint,test:unit,tes... ✅ Succeeded 5m 25s View ↗
nx run-many --target=build --exclude=examples/*... ✅ Succeeded 1m 22s View ↗

☁️ Nx Cloud last updated this comment at 2025-10-19 21:53:14 UTC

@pkg-pr-new
Copy link

pkg-pr-new bot commented Oct 19, 2025

More templates

@tanstack/arktype-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/arktype-adapter@5541

@tanstack/directive-functions-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/directive-functions-plugin@5541

@tanstack/eslint-plugin-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/eslint-plugin-router@5541

@tanstack/history

npm i https://pkg.pr.new/TanStack/router/@tanstack/history@5541

@tanstack/nitro-v2-vite-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/nitro-v2-vite-plugin@5541

@tanstack/react-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router@5541

@tanstack/react-router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router-devtools@5541

@tanstack/react-router-ssr-query

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router-ssr-query@5541

@tanstack/react-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start@5541

@tanstack/react-start-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-client@5541

@tanstack/react-start-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-server@5541

@tanstack/router-cli

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-cli@5541

@tanstack/router-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-core@5541

@tanstack/router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-devtools@5541

@tanstack/router-devtools-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-devtools-core@5541

@tanstack/router-generator

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-generator@5541

@tanstack/router-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-plugin@5541

@tanstack/router-ssr-query-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-ssr-query-core@5541

@tanstack/router-utils

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-utils@5541

@tanstack/router-vite-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-vite-plugin@5541

@tanstack/server-functions-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/server-functions-plugin@5541

@tanstack/solid-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router@5541

@tanstack/solid-router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router-devtools@5541

@tanstack/solid-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start@5541

@tanstack/solid-start-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-client@5541

@tanstack/solid-start-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-server@5541

@tanstack/start-client-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-client-core@5541

@tanstack/start-plugin-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-plugin-core@5541

@tanstack/start-server-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-server-core@5541

@tanstack/start-static-server-functions

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-static-server-functions@5541

@tanstack/start-storage-context

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-storage-context@5541

@tanstack/valibot-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/valibot-adapter@5541

@tanstack/virtual-file-routes

npm i https://pkg.pr.new/TanStack/router/@tanstack/virtual-file-routes@5541

@tanstack/zod-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/zod-adapter@5541

commit: 3d15cc7

@schiller-manuel schiller-manuel merged commit 015bc2d into main Oct 19, 2025
6 checks passed
@schiller-manuel schiller-manuel deleted the fix-5535 branch October 19, 2025 21:56
naoya7076 pushed a commit to naoya7076/router that referenced this pull request Feb 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SSR status code is always 200 even when error throws in loader

1 participant