Skip to content

Conversation

@s3847243
Copy link

@s3847243 s3847243 commented Nov 5, 2025

Fixes #5473:
API routes without a GET handler returned 200 + SPA shell instead of 404 JSON.

Previously:

  • Visiting /api/xyz where only a POST handler existed returned 200 OK with the HTML shell.

Now:

  • Routes without a matching method (and no ANY fallback) correctly return 404 JSON ({ "error": "Not Found" }).
  • HEAD requests behave correctly (empty 404 if no GET handler).

Summary by CodeRabbit

  • New Features

    • Improved HTTP method handling: normalized method matching and automatic HEAD→GET mapping when GET exists.
  • Bug Fixes

    • Clearer 404 behavior: HEAD returns an empty response when unmapped; other methods return JSON { error: "Not Found" } and stop processing.
  • Tests

    • Added unit tests covering method resolution, HEAD behavior, ANY-handler fallback, and missing-handler cases.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 5, 2025

Walkthrough

Normalizes HTTP handler keys, maps HEAD to GET when appropriate, adds ANY-handler awareness, and returns 404 responses when no matching handler exists. Adds unit tests and test mocks, and wires mock modules into the package's Vite test config.

Changes

Cohort / File(s) Summary
Core handler logic update
packages/start-server-core/src/createStartHandler.ts
Normalize handler keys to uppercase, treat HEAD as GET when GET exists, consult ANY fallback, and return early with 404 (special-casing HEAD response body) when no handler is found.
Unit tests
packages/start-server-core/tests/createStartHandler.test.ts
New tests covering GET/POST/HEAD/ANY resolution and 404 behavior across method/handler configurations and SPA fallback simulation.
Test mocks
packages/start-server-core/tests/mocks/injected-head-scripts.ts, packages/start-server-core/tests/mocks/router-entry.ts, packages/start-server-core/tests/mocks/start-entry.ts, packages/start-server-core/tests/mocks/start-manifest.ts
New mock modules: empty injectedHeadScripts export, mutable currentHandlers and fake router, startInstance.getOptions() stub, and a minimal start manifest helper.
Vite test config
packages/start-server-core/vite.config.ts
Add resolve.alias mappings to point virtual/module IDs to the new test mock files (uses node:path and path.resolve for aliases).

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Server as createStartHandler
    participant Lookup as Handler Lookup (normalized)
    participant HandlerFn as Selected Handler
    participant Response

    Client->>Server: HTTP Request (method, path)
    Server->>Lookup: Normalize handlers keys → UPPERCASE
    Server->>Lookup: Lookup by method
    alt method is HEAD and no HEAD but GET exists
        Lookup-->>Server: select GET handler
    else no method -> try ANY
        Lookup-->>Server: select ANY handler (if present)
    end

    alt Handler found
        Server->>HandlerFn: invoke handler
        HandlerFn-->>Response: handler response
        Response-->>Client: 200/handler-defined
    else No handler
        alt request method is HEAD
            Server->>Response: 404 with empty JSON content-type
        else
            Server->>Response: 404 with { error: "Not Found" } JSON body
        end
        Response-->>Client: 404
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Areas to review closely:
    • Uppercasing/normalization logic for handler keys vs. existing registrations
    • HEAD→GET mapping and precedence relative to ANY
    • 404 response shape and content-type for HEAD vs other methods
    • Vite alias additions to ensure test imports resolve correctly

Possibly related PRs

Suggested reviewers

  • schiller-manuel

Poem

🐇
Ears twitch, I hop to test and see,
Methods aligned — HEAD follows GET for free,
ANY listens when others hide,
404s now honest, nothing to bide,
Mocks in burrows, tests pass with glee.

Pre-merge checks and finishing touches

✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main fix: returning 404 for API routes without a GET handler, which is the core objective of the PR.
Linked Issues check ✅ Passed The PR implementation addresses all coding requirements from issue #5473: returning 404 status with JSON body for missing handlers, handling HEAD requests correctly, and preventing the SPA HTML shell from being returned for API routes without matching methods.
Out of Scope Changes check ✅ Passed All changes are directly scoped to the fix: handler normalization, HEAD request mapping, and 404 logic in createStartHandler; comprehensive test coverage; and test infrastructure setup via mock files and Vite config aliases.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 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 16d4a8a and a5d9c70.

📒 Files selected for processing (3)
  • packages/start-server-core/tests/createStartHandler.test.ts (1 hunks)
  • packages/start-server-core/tests/mocks/router-entry.ts (1 hunks)
  • packages/start-server-core/vite.config.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

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

Files:

  • packages/start-server-core/tests/createStartHandler.test.ts
  • packages/start-server-core/tests/mocks/router-entry.ts
  • packages/start-server-core/vite.config.ts
packages/{*-start,start-*}/**

📄 CodeRabbit inference engine (AGENTS.md)

Name and place Start framework packages under packages/-start/ or packages/start-/

Files:

  • packages/start-server-core/tests/createStartHandler.test.ts
  • packages/start-server-core/tests/mocks/router-entry.ts
  • packages/start-server-core/vite.config.ts
🧠 Learnings (9)
📚 Learning: 2025-10-08T08:11:47.088Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 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/start-server-core/tests/createStartHandler.test.ts
  • packages/start-server-core/tests/mocks/router-entry.ts
  • packages/start-server-core/vite.config.ts
📚 Learning: 2025-11-02T16:16:24.898Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5732
File: packages/start-client-core/src/client/hydrateStart.ts:6-9
Timestamp: 2025-11-02T16:16:24.898Z
Learning: In packages/start-client-core/src/client/hydrateStart.ts, the `import/no-duplicates` ESLint disable is necessary for imports from `#tanstack-router-entry` and `#tanstack-start-entry` because both aliases resolve to the same placeholder file (`fake-start-entry.js`) in package.json during static analysis, even though they resolve to different files at runtime.

Applied to files:

  • packages/start-server-core/tests/createStartHandler.test.ts
  • packages/start-server-core/tests/mocks/router-entry.ts
  • packages/start-server-core/vite.config.ts
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/router-core/** : Keep framework-agnostic core router logic in packages/router-core/

Applied to files:

  • packages/start-server-core/tests/mocks/router-entry.ts
📚 Learning: 2025-10-01T18:30:26.591Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: packages/router-core/src/router.ts:2231-2245
Timestamp: 2025-10-01T18:30:26.591Z
Learning: In `packages/router-core/src/router.ts`, the `resolveRedirect` method intentionally strips the router's origin from redirect URLs when they match (e.g., `https://foo.com/bar` → `/bar` for same-origin redirects) while preserving the full URL for cross-origin redirects. This logic should not be removed or simplified to use `location.publicHref` directly.

Applied to files:

  • packages/start-server-core/tests/mocks/router-entry.ts
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to **/*.{ts,tsx} : Use TypeScript in strict mode with extensive type safety across the codebase

Applied to files:

  • packages/start-server-core/tests/mocks/router-entry.ts
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/{react-router,solid-router}/** : Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/

Applied to files:

  • packages/start-server-core/tests/mocks/router-entry.ts
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/{router-cli,router-generator,router-plugin,virtual-file-routes}/** : Keep CLI, generators, bundler plugins, and virtual file routing utilities in their dedicated tooling package directories

Applied to files:

  • packages/start-server-core/tests/mocks/router-entry.ts
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/{router-devtools,*-router-devtools}/** : Keep router devtools packages in packages/router-devtools/ and packages/*-router-devtools/

Applied to files:

  • packages/start-server-core/tests/mocks/router-entry.ts
📚 Learning: 2025-10-01T18:31:35.420Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: e2e/react-start/custom-basepath/src/routeTree.gen.ts:58-61
Timestamp: 2025-10-01T18:31:35.420Z
Learning: Do not review files named `routeTree.gen.ts` in TanStack Router repositories, as these are autogenerated files that should not be manually modified.

Applied to files:

  • packages/start-server-core/tests/mocks/router-entry.ts
🧬 Code graph analysis (1)
packages/start-server-core/tests/createStartHandler.test.ts (2)
packages/start-server-core/src/createStartHandler.ts (1)
  • createStartHandler (54-341)
packages/start-server-core/tests/mocks/router-entry.ts (1)
  • currentHandlers (3-3)
🔇 Additional comments (3)
packages/start-server-core/tests/createStartHandler.test.ts (1)

1-80: LGTM! Comprehensive test coverage for HTTP method handling.

The test suite thoroughly validates the fix described in the PR objectives:

  • ✅ Returns 404 JSON (not HTML) when GET is absent
  • ✅ Executes matching handlers (POST, GET)
  • ✅ Maps HEAD to GET when available, or returns 404 when not
  • ✅ Falls back to ANY for unsupported methods

The test isolation via beforeEach cleanup and the mock wiring via currentHandlers are well-implemented.

packages/start-server-core/tests/mocks/router-entry.ts (1)

1-30: LGTM! Test mock correctly implements the minimal router interface.

The mock provides the necessary structure to test handler resolution:

  • currentHandlers is correctly exported as a mutable map for test setup
  • getMatchedRoutes returns a structure that binds server.handlers to currentHandlers
  • Type-only import and const declaration follow best practices (past issues resolved)

The minimal implementation is appropriate for the test scenarios and avoids unnecessary complexity.

packages/start-server-core/vite.config.ts (1)

16-24: LGTM! Alias mappings correctly wire test mocks into the environment.

The resolve.alias configuration enables tests to inject mocks for:

  • #tanstack-router-entryrouter-entry.ts
  • #tanstack-start-entrystart-entry.ts
  • tanstack-start-manifest:vstart-manifest.ts
  • tanstack-start-injected-head-scripts:vinjected-head-scripts.ts

This approach cleanly isolates the test environment and allows the test suite to exercise the handler resolution logic without requiring a full router setup.


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 and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6baffeb and 16d4a8a.

📒 Files selected for processing (7)
  • packages/start-server-core/src/createStartHandler.ts (1 hunks)
  • packages/start-server-core/tests/createStartHandler.test.ts (1 hunks)
  • packages/start-server-core/tests/mocks/injected-head-scripts.ts (1 hunks)
  • packages/start-server-core/tests/mocks/router-entry.ts (1 hunks)
  • packages/start-server-core/tests/mocks/start-entry.ts (1 hunks)
  • packages/start-server-core/tests/mocks/start-manifest.ts (1 hunks)
  • packages/start-server-core/vite.config.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

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

Files:

  • packages/start-server-core/tests/mocks/injected-head-scripts.ts
  • packages/start-server-core/tests/mocks/start-entry.ts
  • packages/start-server-core/tests/mocks/router-entry.ts
  • packages/start-server-core/tests/createStartHandler.test.ts
  • packages/start-server-core/tests/mocks/start-manifest.ts
  • packages/start-server-core/src/createStartHandler.ts
  • packages/start-server-core/vite.config.ts
packages/{*-start,start-*}/**

📄 CodeRabbit inference engine (AGENTS.md)

Name and place Start framework packages under packages/-start/ or packages/start-/

Files:

  • packages/start-server-core/tests/mocks/injected-head-scripts.ts
  • packages/start-server-core/tests/mocks/start-entry.ts
  • packages/start-server-core/tests/mocks/router-entry.ts
  • packages/start-server-core/tests/createStartHandler.test.ts
  • packages/start-server-core/tests/mocks/start-manifest.ts
  • packages/start-server-core/src/createStartHandler.ts
  • packages/start-server-core/vite.config.ts
🧠 Learnings (4)
📚 Learning: 2025-10-08T08:11:47.088Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 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/start-server-core/tests/mocks/router-entry.ts
  • packages/start-server-core/tests/createStartHandler.test.ts
  • packages/start-server-core/tests/mocks/start-manifest.ts
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/router-core/** : Keep framework-agnostic core router logic in packages/router-core/

Applied to files:

  • packages/start-server-core/tests/mocks/router-entry.ts
📚 Learning: 2025-11-02T16:16:24.898Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5732
File: packages/start-client-core/src/client/hydrateStart.ts:6-9
Timestamp: 2025-11-02T16:16:24.898Z
Learning: In packages/start-client-core/src/client/hydrateStart.ts, the `import/no-duplicates` ESLint disable is necessary for imports from `#tanstack-router-entry` and `#tanstack-start-entry` because both aliases resolve to the same placeholder file (`fake-start-entry.js`) in package.json during static analysis, even though they resolve to different files at runtime.

Applied to files:

  • packages/start-server-core/tests/mocks/router-entry.ts
  • packages/start-server-core/tests/mocks/start-manifest.ts
  • packages/start-server-core/vite.config.ts
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/{router-cli,router-generator,router-plugin,virtual-file-routes}/** : Keep CLI, generators, bundler plugins, and virtual file routing utilities in their dedicated tooling package directories

Applied to files:

  • packages/start-server-core/vite.config.ts
🧬 Code graph analysis (2)
packages/start-server-core/tests/createStartHandler.test.ts (2)
packages/start-server-core/src/createStartHandler.ts (1)
  • createStartHandler (54-341)
packages/start-server-core/tests/mocks/router-entry.ts (1)
  • currentHandlers (3-3)
packages/start-server-core/src/createStartHandler.ts (1)
packages/start-client-core/src/serverRoute.ts (1)
  • RouteMethod (427-435)
🪛 ESLint
packages/start-server-core/tests/mocks/router-entry.ts

[error] 1-1: All imports in the declaration are only used as types. Use import type.

(@typescript-eslint/consistent-type-imports)


[error] 3-3: 'currentHandlers' is never reassigned. Use 'const' instead.

(prefer-const)

packages/start-server-core/tests/createStartHandler.test.ts

[error] 1-1: Member 'expect' of the import declaration should be sorted alphabetically.

(sort-imports)

packages/start-server-core/vite.config.ts

[error] 7-7: Expected 1 empty line after import statement not followed by another import.

(import/newline-after-import)


[error] 7-7: path import should occur before import of vitest/config

(import/order)


[error] 7-7: Prefer node:path over path.

(node/prefer-node-protocol)

🔇 Additional comments (9)
packages/start-server-core/tests/mocks/start-manifest.ts (1)

1-10: LGTM!

The mock manifest structure is minimal and appropriate for testing the server handler behavior.

packages/start-server-core/tests/mocks/start-entry.ts (1)

1-7: LGTM!

The mock start entry provides minimal defaults suitable for testing the start handler.

packages/start-server-core/tests/mocks/injected-head-scripts.ts (1)

1-1: LGTM!

The empty string mock for injected head scripts is appropriate for isolated testing.

packages/start-server-core/src/createStartHandler.ts (2)

383-392: LGTM! Correct HTTP method normalization and HEAD mapping.

The normalization ensures case-insensitive method matching, and the HEAD→GET mapping follows HTTP semantics correctly.


395-408: LGTM! Proper 404 handling for routes without matching handlers.

The logic correctly returns 404 JSON responses when no handler exists for the requested method and no ANY fallback is present. HEAD requests appropriately return an empty body with 404 status per HTTP standards.

packages/start-server-core/vite.config.ts (1)

15-23: LGTM! Test mock aliases properly configured.

The path aliases enable isolated testing of the start handler by redirecting module imports to test mocks.

packages/start-server-core/tests/createStartHandler.test.ts (2)

19-33: LGTM! Core test validates 404 JSON response.

This test correctly validates the main fix: routes without a GET handler now return 404 JSON instead of 200 OK with the HTML shell.


35-80: LGTM! Comprehensive method resolution coverage.

The test suite thoroughly validates POST handling, HEAD→GET mapping, HEAD 404 behavior, and ANY handler fallback scenarios.

packages/start-server-core/tests/mocks/router-entry.ts (1)

5-30: LGTM! Minimal router mock for testing.

The fake router provides all necessary properties to exercise the handler resolution logic without requiring the full router implementation.

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.

API routes without GET handlers doesn't return a 404 status code

1 participant