Skip to content

Add guidance for invalid route return values#157

Open
momenbuilds wants to merge 2 commits into
tscircuit:mainfrom
momenbuilds:codex/invalid-route-return-guidance
Open

Add guidance for invalid route return values#157
momenbuilds wants to merge 2 commits into
tscircuit:mainfrom
momenbuilds:codex/invalid-route-return-guidance

Conversation

@momenbuilds
Copy link
Copy Markdown

Summary

  • Add an early invalid-return guard for route handlers that return null, primitives, or functions.
  • Surface actionable guidance to use ctx.json({...}) or return a Response instead of letting low-level response serialization fail.
  • Add regression coverage for undefined, null, string, number, boolean, symbol, bigint, and function returns while preserving the existing raw object test.

Validation

  • npm test -- tests/errors/do-not-allow-raw-json.test.ts
  • npm run lint -- src/create-with-winter-spec.ts tests/errors/do-not-allow-raw-json.test.ts
  • git diff --check

/claim #30

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR strengthens response serialization by explicitly rejecting primitive return values from route handlers and adds tests to ensure users get an actionable error message.

Changes:

  • Add a runtime guard in response serialization to throw a clearer error when a handler returns a primitive (or null) directly.
  • Add parameterized tests covering several invalid primitive return types (e.g., undefined, string, number, symbol, bigint).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
tests/errors/do-not-allow-raw-json.test.ts Adds a table-driven test suite to verify actionable errors for primitive handler returns.
src/create-with-winter-spec.ts Adds a primitive/null guard in serializeResponse with an improved error message.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/create-with-winter-spec.ts Outdated
if (rawResponse === null || typeof rawResponse !== "object") {
const returnedType = rawResponse === null ? "null" : typeof rawResponse
throw new Error(
`Use ctx.json({...}) or return a Response instead of returning ${returnedType} directly.`
Comment on lines +58 to +64
async (req, ctx, next) => {
try {
return await next(req, ctx)
} catch (e: any) {
return Response.json({ error: e.message }, { status: 500 })
}
},
@momenbuilds
Copy link
Copy Markdown
Author

Follow-up pushed in 809c6b3 addressing automated review feedback: the primitive-return guidance now says ctx.json(value) instead of implying only object literals, and the test middleware now handles non-Error throws safely. Local validation: npm test -- tests/errors/do-not-allow-raw-json.test.ts, npm run lint -- src/create-with-winter-spec.ts tests/errors/do-not-allow-raw-json.test.ts, git diff --check. CI is green again.

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.

2 participants