Skip to content

fix(nextjs): fix JSON parsing in Pages Router handler#173

Merged
halvaradop merged 1 commit into
masterfrom
fix/json-parsing
May 28, 2026
Merged

fix(nextjs): fix JSON parsing in Pages Router handler#173
halvaradop merged 1 commit into
masterfrom
fix/json-parsing

Conversation

@halvaradop
Copy link
Copy Markdown
Member

@halvaradop halvaradop commented May 28, 2026

Description

This pull request fixes JSON body handling in the Pages Router toHandler utility. Previously, when creating a Request object from the Next.js Pages Router request, the body was always serialized using JSON.stringify. However, in the Pages Router runtime, req.body is already parsed by Next.js when the request content type is JSON.

Because of this, the body was being serialized twice, causing invalid request payloads in endpoints that rely on request bodies, such as:

  • PATCH /session
  • POST /signin/credentials

This update removes the unnecessary JSON.stringify call and passes the parsed body directly to the Request constructor.

Technical Details

Next.js Pages Router automatically parses JSON request bodies by default, as documented by Next.js.

The previous implementation incorrectly assumed the body was still a raw payload that required serialization before constructing the Request object.

- body: method !== "GET" && method !== "HEAD" && req.body ? JSON.stringify(req.body) : undefined,
+body: method !== "GET" && method !== "HEAD" ? req.body : undefined,

@vercel
Copy link
Copy Markdown
Contributor

vercel Bot commented May 28, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
auth Skipped Skipped May 28, 2026 10:07pm

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 28, 2026

📝 Walkthrough

Walkthrough

This PR refactors the Next.js Pages Router auth handler to export setResponseHeaders with improved Set-Cookie handling, changes request construction to use Headers directly, and simplifies response handling by always parsing and returning JSON responses instead of special-casing redirects. Comprehensive tests, test infrastructure, and client flow updates complete the integration.

Changes

Auth Handler Refactor and Integration Testing

Layer / File(s) Summary
Handler response headers and request construction
packages/next/src/pages/handler.ts
setResponseHeaders is exported and updated to handle Set-Cookie via headers.getSetCookie(). toHandler changes request header construction to directly wrap req.headers as a Headers instance, simplifies body handling, removes special redirect handling, and always parses responses as JSON via res.status(...).json(...).
Test infrastructure and configuration
packages/next/vitest.config.ts, packages/next/tsconfig.json, packages/next/package.json
Vitest configuration is created with secret/salt environment injection, path aliases, and coverage settings. TypeScript config is updated with @test path alias and test directory inclusion. Package scripts for test execution and coverage are added, along with node-mocks-http development dependency.
Auth preset and handler test suite
packages/next/test/pages/preset.ts, packages/next/test/pages/handler.test.ts
Auth test preset defines credential-based authentication with username/password validation and email generation. Test helper createHandler wraps handler invocation with mock req/res objects. Test suite covers handler method validation, session retrieval (HTTP and HTTPS cookies), credential sign-in (valid and invalid payloads with schema validation), session updates with CSRF protection, and sign-out flows. Additional tests verify setResponseHeaders correctly applies headers and preserves Set-Cookie arrays.
Client page OAuth and sign-out flow updates
apps/nextjs/pages-router/src/pages/client/index.tsx
Sign-out now redirects to /server instead of /client. OAuth provider button click calls signIn(..., { redirect: true }) to enable redirect behavior after authentication.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • aura-stack-ts/auth#172: Updates OAuth sign-in redirect behavior in the app-router client page to use signIn(..., { redirect: true }), mirroring the pages-router changes in this PR.
  • aura-stack-ts/auth#169: Introduced the Pages Router adapter toHandler that this PR refactors—directly modified in handler response/header behavior, Set-Cookie handling, and JSON parsing logic.
  • aura-stack-ts/auth#143: Standardized API and client return types (toResponse() and redirect contracts) that this PR's handler changes now fully implement by always parsing JSON and removing redirect special-casing.

Suggested labels

refactor

Poem

🐰 Hop skip and a test bound,
Response headers found,
Cookies preserved true,
JSON all the way through,
Auth flows that leap and abound! 🚀

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'fix(nextjs): fix JSON parsing in Pages Router handler' clearly describes a key change in the handler.ts file where JSON parsing logic was updated, making it directly relevant to the main changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/json-parsing

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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
Copy Markdown

@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: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/next/src/pages/handler.ts`:
- Around line 59-60: In toHandler, avoid unguarded response.json() for upstream
responses by checking response.status and response.headers before parsing: if
status is 204/304 or content-length is 0 or content-type is missing/not
application/json, skip calling response.json() and forward an empty body or raw
text (use response.text()) instead; otherwise parse JSON and call
res.status(response.status).json(data). Update the logic around the existing
response and res handling in toHandler to inspect response.status and
response.headers.get('content-type')/content-length and branch accordingly so
non-JSON/empty upstream bodies don't cause a thrown error turned into a 500.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b0d6ac88-1365-45ae-a924-851391d1f2ca

📥 Commits

Reviewing files that changed from the base of the PR and between 9e4e472 and 0c4ba1c.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (7)
  • apps/nextjs/pages-router/src/pages/client/index.tsx
  • packages/next/package.json
  • packages/next/src/pages/handler.ts
  • packages/next/test/pages/handler.test.ts
  • packages/next/test/pages/preset.ts
  • packages/next/tsconfig.json
  • packages/next/vitest.config.ts

Comment thread packages/next/src/pages/handler.ts
@halvaradop halvaradop merged commit dfb5615 into master May 28, 2026
7 checks passed
@halvaradop halvaradop deleted the fix/json-parsing branch May 28, 2026 22:36
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.

1 participant