Uh oh!
There was an error while loading. Please reload this page.
fix(backend): stop authenticateRequest from consuming the request body - #8708
Conversation
🦋 Changeset detectedLatest commit: b21dca3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Snapi: no API changes detected in |
@clerk/astro@clerk/backend@clerk/chrome-extension@clerk/clerk-js@clerk/expo@clerk/expo-passkeys@clerk/express@clerk/fastify@clerk/hono@clerk/localizations@clerk/nextjs@clerk/nuxt@clerk/react@clerk/react-router@clerk/shared@clerk/tanstack-react-start@clerk/testing@clerk/ui@clerk/upgrade@clerk/vuecommit: |
📝 WalkthroughWalkthroughThis PR fixes a bug where Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
87212c5 to
b21dca3CompareThere was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/backend/src/tokens/__tests__/clerkRequest.test.ts (1)
7-18: 💤 Low valueConsider adding an explicit return type annotation for clarity.
While the return type is easily inferred as
boolean, adding an explicit annotation would improve readability and document the intent.📝 Optional enhancement
-const supportsStreamConstruction = (() => {+const supportsStreamConstruction = ((): boolean => { try { new ReadableStream({🤖 Prompt for 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. In `@packages/backend/src/tokens/__tests__/clerkRequest.test.ts` around lines 7 - 18, The IIFE assigned to supportsStreamConstruction should have an explicit boolean return type for clarity; update the constant declaration for supportsStreamConstruction (the immediately-invoked function expression that constructs a ReadableStream) to include a : boolean return type annotation so the intent is documented and the inferred type is explicit.
🤖 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.
Nitpick comments:
In `@packages/backend/src/tokens/__tests__/clerkRequest.test.ts`:
- Around line 7-18: The IIFE assigned to supportsStreamConstruction should have
an explicit boolean return type for clarity; update the constant declaration for
supportsStreamConstruction (the immediately-invoked function expression that
constructs a ReadableStream) to include a : boolean return type annotation so
the intent is documented and the inferred type is explicit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 1cd5ed60-34a6-444e-b621-8de615e4db09
📒 Files selected for processing (3)
.changeset/clerkrequest-omit-body.mdpackages/backend/src/tokens/__tests__/clerkRequest.test.tspackages/backend/src/tokens/clerkRequest.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/backend/src/tokens/clerkRequest.ts
- .changeset/clerkrequest-omit-body.md
wobsoriano
left a comment
There was a problem hiding this comment.
looks good! thanks for fixing 🫡
ClerkRequestclones the incoming request to read headers, cookies, and URL, but the clone was also forwarding the body. On Node/undici that body is a single-use stream shared with the original, so once anything reads the clone the original throws "Body is unusable" downstream (#8305, e.g. a Hono POST handler callingc.req.json()).The clone now hides
bodynext tosignalin the existing Proxy. I kept the Proxy rather than an explicit RequestInit on purpose: that shape was reverted in e9f8d1a because eagerly readingcachebreaks Cloudflare Workers. Auth never touches the body, so dropping it from the clone is safe. Theretains the bodytest asserted the old behavior and is now a streaming-body regression test.Closes#8305.