fix: preserve middleware headers for Array subclass responses#1843
fix: preserve middleware headers for Array subclass responses#1843dcitron-espoc wants to merge 2 commits intoelysiajs:mainfrom
Conversation
When a handler returns an Array subclass (e.g. postgres.js RowList, Bun.sql results), the constructor.name check in mapResponse and mapEarlyResponse does not match "Array", falling through to the default case. The default case's Array.isArray() fallback then created a Response without passing the `set` object, silently discarding all middleware-set headers (CORS, cache-control, etc.). The JSON body was serialized correctly, but headers were lost — causing browsers to reject responses with misleading CORS errors while the server logged no errors. This is the header-loss half of the bug reported in elysiajs#1656. PR elysiajs#1675 fixed the body serialization but did not address header propagation. Fix: pass `set` to Response.json() (bun adapter) and new Response(JSON.stringify(), set) (web-standard adapter) in all Array.isArray() fallback paths within mapResponse and mapEarlyResponse. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughArray-subclass responses (e.g., postgres.js Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/adapter/bun/handler.ts (1)
129-131: Tiny smug nudge: extract this Array-subclass fallback into one helper, baka~Same logic is repeated in three branches; centralizing it would reduce drift and prevent this exact regression from sneaking back again ♡
Also applies to: 269-271, 380-382
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/adapter/bun/handler.ts` around lines 129 - 131, Extract the repeated Array-subclass fallback into a single helper (e.g., respondWithArray or sendArrayResponse) that takes (response, set) and returns the Response.json(...) result; then replace the three duplicated blocks (the Array.isArray(response) checks in handler.ts — the branches around the current occurrences at the shown locations) with calls to that helper to centralize the behavior and avoid drift.src/adapter/web-standard/handler.ts (1)
163-170: One more polish, genius~ unify this into a shared helper before it drifts again (¬、¬)The repeated Array-subclass serialization block in three places is brittle; a helper for “array-like JSON response with
set” would improve maintainability and consistency.Also applies to: 321-328, 454-461
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/adapter/web-standard/handler.ts` around lines 163 - 170, Create a small helper (e.g., serializeArrayResponse(set, response)) that encapsulates the Array-like response logic: ensure set.headers['content-type'] ||= 'application/json' and return new Response(JSON.stringify(response), set). Replace the duplicated blocks in handler.ts (the current Array.isArray(response) branch and the similar blocks at the other two locations referenced) with calls to this helper so array-subclass/RowList serialization is consistent and maintained in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/adapter/bun/handler.ts`:
- Around line 129-131: Extract the repeated Array-subclass fallback into a
single helper (e.g., respondWithArray or sendArrayResponse) that takes
(response, set) and returns the Response.json(...) result; then replace the
three duplicated blocks (the Array.isArray(response) checks in handler.ts — the
branches around the current occurrences at the shown locations) with calls to
that helper to centralize the behavior and avoid drift.
In `@src/adapter/web-standard/handler.ts`:
- Around line 163-170: Create a small helper (e.g., serializeArrayResponse(set,
response)) that encapsulates the Array-like response logic: ensure
set.headers['content-type'] ||= 'application/json' and return new
Response(JSON.stringify(response), set). Replace the duplicated blocks in
handler.ts (the current Array.isArray(response) branch and the similar blocks at
the other two locations referenced) with calls to this helper so
array-subclass/RowList serialization is consistent and maintained in one place.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a8a29463-016f-454a-afbe-3d3c03c308a1
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
src/adapter/bun/handler.tssrc/adapter/web-standard/handler.tstest/response/headers.test.ts
Address CodeRabbit review: the Array.isArray() fallback was repeated 3 times per adapter file. Extract into a shared helper to prevent drift and keep the fix centralized. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
Fixes #1842 — Array subclass responses (e.g. postgres.js
RowList, Bun SQL results) silently drop all middleware-set headers (CORS, cache-control, custom headers).This is the header-loss half of #1656. PR #1675 fixed body serialization by adding an
Array.isArray()fallback in thedefaultcase, but that fallback calledResponse.json(response)/new Response(JSON.stringify(response), { headers: ... })without passingset, discarding all headers that middleware had added.Changes
src/adapter/bun/handler.ts— PasssettoResponse.json()in 3Array.isArray()fallback paths:src/adapter/web-standard/handler.ts— Usesetinstead of constructing new headers in 3Array.isArray()fallback paths:test/response/headers.test.ts— 2 regression tests:preserve headers when handler returns Array subclass— verifies custom headers + content-type survivepreserve status when handler returns Array subclass— verifies status code survivesTest plan
RowListresponses now include CORS headers when using@elysiajs/corsSummary by CodeRabbit
Bug Fixes
Tests