fix: handle synchronous handler/middleware#5270
Conversation
WalkthroughAdds try/catch around middleware execution, captures and merges middleware results into context via handleCtxResult, integrates next() results similarly, and handles “special” error-responses by converting them into a response result instead of throwing; non-special errors continue to throw. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant R as Request
participant H as StartHandler
participant M as Middleware
participant N as next()
participant C as handleCtxResult
participant S as isSpecialResponse
R->>H: invoke handler
H->>M: call middleware(ctx, next)
rect rgba(200,230,255,0.3)
note over H,M: New try/catch around middleware chain
M->>N: invoke next()
N-->>M: nextResult
M->>C: handleCtxResult(ctx, nextResult)
C-->>M: mergedCtx
end
M-->>H: result or mergedCtx
H->>C: handleCtxResult(ctx, result)
C-->>H: finalCtx/response
H-->>R: response
opt Error thrown in middleware/next
H->>S: isSpecialResponse(error)
alt Special response
note over H: Convert error to response result
H->>C: handleCtxResult(ctx, responseFromError)
C-->>H: finalCtx/response
H-->>R: response
else Non-special error
H-->>R: throw error
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
|
View your CI Pipeline Execution ↗ for commit 07c412e
☁️ Nx Cloud last updated this comment at |
More templates
@tanstack/arktype-adapter
@tanstack/directive-functions-plugin
@tanstack/eslint-plugin-router
@tanstack/history
@tanstack/nitro-v2-vite-plugin
@tanstack/react-router
@tanstack/react-router-devtools
@tanstack/react-router-ssr-query
@tanstack/react-start
@tanstack/react-start-client
@tanstack/react-start-server
@tanstack/router-cli
@tanstack/router-core
@tanstack/router-devtools
@tanstack/router-devtools-core
@tanstack/router-generator
@tanstack/router-plugin
@tanstack/router-ssr-query-core
@tanstack/router-utils
@tanstack/router-vite-plugin
@tanstack/server-functions-plugin
@tanstack/solid-router
@tanstack/solid-router-devtools
@tanstack/solid-start
@tanstack/solid-start-client
@tanstack/solid-start-server
@tanstack/start-client-core
@tanstack/start-plugin-core
@tanstack/start-server-core
@tanstack/start-static-server-functions
@tanstack/start-storage-context
@tanstack/valibot-adapter
@tanstack/virtual-file-routes
@tanstack/zod-adapter
commit: |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/start-server-core/src/createStartHandler.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/src/createStartHandler.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/src/createStartHandler.ts
🪛 Biome (2.1.2)
packages/start-server-core/src/createStartHandler.ts
[error] 519-519: Catch clause variable type annotation must be 'any' or 'unknown' if specified.
(parse)
⏰ 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: Preview
- GitHub Check: Test
| } catch (err: TODO) { | ||
| if (isSpecialResponse(err)) { | ||
| return { | ||
| result = { | ||
| response: err, | ||
| } | ||
| } else { | ||
| throw err | ||
| } | ||
|
|
||
| throw err | ||
| }) | ||
| } |
There was a problem hiding this comment.
Fix lint failure in catch clause
Biome reports “Catch clause variable type annotation must be 'any' or 'unknown'” for catch (err: TODO), which breaks lint and will block CI. Please swap the annotation to unknown (or remove it) so the pipeline passes.
- } catch (err: TODO) {
+ } catch (err: unknown) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (err: TODO) { | |
| if (isSpecialResponse(err)) { | |
| return { | |
| result = { | |
| response: err, | |
| } | |
| } else { | |
| throw err | |
| } | |
| throw err | |
| }) | |
| } | |
| } catch (err: unknown) { | |
| if (isSpecialResponse(err)) { | |
| result = { | |
| response: err, | |
| } | |
| } else { | |
| throw err | |
| } | |
| } |
🧰 Tools
🪛 Biome (2.1.2)
[error] 519-519: Catch clause variable type annotation must be 'any' or 'unknown' if specified.
(parse)
🤖 Prompt for AI Agents
In packages/start-server-core/src/createStartHandler.ts around lines 519 to 527,
the catch clause currently uses an invalid annotation `catch (err: TODO)` which
triggers the Biome lint rule requiring catch variables be typed as 'any' or
'unknown'; change the annotation to `catch (err: unknown)` (or remove the type
entirely) so the catch variable complies with lint, then ensure subsequent uses
of `err` are narrowed (e.g., use type guards like isSpecialResponse(err)) or
properly asserted before treating it as a specific type.
Summary by CodeRabbit
Bug Fixes
Reliability