Skip to content

Conversation

@azrikahar
Copy link
Contributor

@azrikahar azrikahar commented Sep 7, 2025

πŸ”— Linked issue

❓ Type of change

  • πŸ“– Documentation (updates to the documentation, readme, or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

If the route starts with /api/ (either because it is placed in api/ or routes/api/) the default will change to application/json and a JSON object will be sent.

I believe the above excerpt is no longer applicable due to the changes from #3002.

It was true when the response still uses a util function called isJsonRequest and it has a specific check for event.path.startsWith("/api/") to make sure routes in that path will return JSON:

export function isJsonRequest(event: H3Event) {
// If the client specifically requests HTML, then avoid classifying as JSON.
if (hasReqHeader(event, "accept", "text/html")) {
return false;
}
return (
hasReqHeader(event, "accept", "application/json") ||
hasReqHeader(event, "user-agent", "curl/") ||
hasReqHeader(event, "user-agent", "httpie/") ||
hasReqHeader(event, "sec-fetch-mode", "cors") ||
event.path.startsWith("/api/") ||
event.path.endsWith(".json")
);
}

but as it was removed and #3002 specifically split the error handler into dev and prod handlers, I've opted to update the docs based on the new error handling changes.

That being said, should the following existing sentence:

This behaviour can be overridden by some request properties (e.g.: Accept or User-Agent headers).

be removed since it might be redundant now? or at least the User-Agent part might not really work, assuming if I didn't misunderstand the new error handling logic of course.

Update: in hindsight maybe this PR should've targeted v2 instead of v3 since #3002 was merged into v2. Do let me know if you'd prefer that.

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@azrikahar azrikahar requested a review from pi0 as a code owner September 7, 2025 04:02
@vercel
Copy link

vercel bot commented Sep 7, 2025

@azrikahar is attempting to deploy a commit to the Nitro Team on Vercel.

A member of the Team first needs to authorize it.

@azrikahar azrikahar changed the base branch from v3 to v2 September 7, 2025 04:03
@azrikahar azrikahar changed the base branch from v2 to v3 September 7, 2025 04:04
Copy link
Member

@pi0 pi0 left a comment

Choose a reason for hiding this comment

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

Thanks ❀️

@coderabbitai
Copy link

coderabbitai bot commented Dec 16, 2025

Caution

Review failed

The pull request is closed.

πŸ“ Walkthrough

Walkthrough

Documentation update clarifying error handling behavior based on environment. In development, HTML error pages are served for Accept: text/html requests; in production, errors are always sent as JSON. Behavior can be overridden by request properties. Explanatory examples adjusted accordingly.

Changes

Cohort / File(s) Change Summary
Error Handling Documentation
docs/1.docs/5.routing.md
Updated error handling description to reflect environment-based behavior (development vs. production) instead of route-path-based defaults. Clarified HTML vs. JSON response handling and added note about request property overrides.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Documentation-only change with conceptual clarity updates
  • Error handling behavior explanation requires verification against actual implementation behavior
  • Examples need review to ensure they accurately reflect the documented behavior
✨ Finishing touches
πŸ§ͺ 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 bad377c and ac3a32f.

πŸ“’ Files selected for processing (1)
  • docs/1.docs/5.routing.md (1 hunks)

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.

@pi0 pi0 merged commit a0622a9 into nitrojs:main Dec 16, 2025
6 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants