Skip to content

Fix issue: When a user returns return { ... } from a route instead of return ctx.json({ ... }), throw an er#149

Open
DoView1 wants to merge 2 commits into
tscircuit:mainfrom
DoView1:codex-maintenance-experiment_mpch5bip_g41bqcul
Open

Fix issue: When a user returns return { ... } from a route instead of return ctx.json({ ... }), throw an er#149
DoView1 wants to merge 2 commits into
tscircuit:mainfrom
DoView1:codex-maintenance-experiment_mpch5bip_g41bqcul

Conversation

@DoView1
Copy link
Copy Markdown

@DoView1 DoView1 commented May 19, 2026

Addresses #30

Summary

  • Adds a shared raw-return guard so accidental raw object responses produce the intended ctx.json({...}) guidance.
  • Covers route middleware returning raw objects before the final route handler path.
  • Adds the follow-up null regression handling requested by automated review.

Verification

  • Targeted harness confirmed route handler and route middleware raw returns produce Use ctx.json({...}) instead of returning an object directly.
  • Raw Response returns still pass through successfully.
  • Full npm test / npm run build were blocked locally by pre-existing dependency/setup issues (del missing and lockfile out of sync), so the PR includes focused regression evidence rather than a full-suite pass.

/claim #30

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a4da75b979

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

"Use ctx.json({...}) instead of returning an object directly."

export const isRawObjectResponse = (response: unknown) =>
response !== null &&
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Handle null raw responses before status access

When a handler or route middleware returns null (for example, raw JSON null instead of ctx.json(null)), this predicate returns false, so serializeResponse falls through to rawResponse.status and throws a TypeError rather than the intended raw-response error. The previous check treated null as an invalid raw object response, so this regression leaves one raw JSON value with an unhelpful 500; keep null in the raw-response path or special-case it before dereferencing.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed in 63a5cd2: null raw returns now stay on the raw-response guidance path before any .status access, so this should no longer fall through to a TypeError.

@DoView1
Copy link
Copy Markdown
Author

DoView1 commented May 27, 2026

Quick follow-up: I pushed 63a5cd2 to address the automated review note about null raw responses before status access. The PR now includes both the middleware raw-object case and the null regression path, and GitHub reports the branch as mergeable.

Happy to narrow or adjust the behavior if the intended fix for #30 should prefer a different boundary.

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.

1 participant