Fix negative response MIME type for arrayResultHandler
#2857
Conversation
WalkthroughThe changes update the negative response MIME type for the deprecated Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~6 minutes Suggested labels
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
CHANGELOG.md (1)
7-8: Minor grammar touch-up for clarityConsider replacing “Should have been” with “It should be” (or “It should have been”) so the sentence reads more naturally.
- - Should have been `text/plain`. + - It should have been `text/plain`.example/example.documentation.yaml (1)
458-465: HEAD responses generally shouldn’t declare a body
HEAD must not send a payload, yet the spec advertisestext/plain.
If the implementation indeed returns an empty body (typical), consider dropping the entirecontentblock to avoid misleading clients.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
CHANGELOG.md(1 hunks)example/example.documentation.yaml(2 hunks)express-zod-api/src/result-handler.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
express-zod-api/src/result-handler.ts (4)
Learnt from: RobinTail
PR: #2546
File: express-zod-api/src/io-schema.ts:5-8
Timestamp: 2025-05-28T05:04:40.327Z
Learning: The Base type object & { [Symbol.iterator]?: never } in express-zod-api correctly excludes arrays and other iterables from IOSchema. Arrays are properly rejected because they have a defined Symbol.iterator method which conflicts with the never constraint, as demonstrated by TypeScript compiler errors when attempting to assign z.array() to IOSchema.
Learnt from: RobinTail
PR: #2546
File: express-zod-api/src/json-schema-helpers.ts:75-87
Timestamp: 2025-05-27T20:40:19.548Z
Learning: In express-zod-api's flattenIO function in json-schema-helpers.ts, the additionalProperties field is used as a template to generate property schemas for literal property names extracted from propertyNames.const and propertyNames.enum. Converting boolean additionalProperties values to empty objects {} via Object(entry.additionalProperties) is intentional behavior, as the function only needs property schema templates, not the boolean semantics of additionalProperties.
Learnt from: RobinTail
PR: #2736
File: express-zod-api/tsup.config.ts:12-26
Timestamp: 2025-06-14T16:42:52.972Z
Learning: In express-zod-api tsup configurations, the direct mutation of options.supported in the esbuildOptions callback is intentional behavior and should not be flagged as a side effect issue.
Learnt from: RobinTail
PR: #2546
File: express-zod-api/src/documentation-helpers.ts:508-512
Timestamp: 2025-05-28T07:58:09.853Z
Learning: In express-zod-api, when working with Zod's JSON schema override callbacks, using delete to mutate ctx.jsonSchema is the recommended approach per Zod's official documentation, even if it triggers performance linting warnings. This is preferable to creating copies with undefined values, especially for snapshot testing.
CHANGELOG.md (1)
Learnt from: RobinTail
PR: #2546
File: express-zod-api/tests/form-schema.spec.ts:31-31
Timestamp: 2025-05-27T19:27:13.492Z
Learning: Zod version 3.25.0 and later expose the Zod v4 API through the special import paths "zod/v4" and "zod/v4/core", allowing v4 features like .loose() to be used even when the package.json dependency shows a 3.x version.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: report
🔇 Additional comments (2)
express-zod-api/src/result-handler.ts (1)
172-175: LGTM! Metadata now aligns with implementation behavior.The change correctly updates the negative response metadata to explicitly specify
text/plainMIME type, which matches the actual handler implementation that uses.type("text/plain")and.send()on lines 182-183. This ensures proper OpenAPI documentation generation.example/example.documentation.yaml (1)
442-449: Confirmtext/plainonly for deprecated arrayResultHandler endpoints
The switch totext/plainfor the GET /v1/user/list 400-response is correct. All other 4xx responses still useapplication/json, which is expected for endpoints that return structured error objects.Please double-check that no other endpoints backed by the now-deprecated
arrayResultHandlerstill expose errors asapplication/json. You can search your codebase for the handler’s usage and cross-reference those routes in your documentation.• example/example.documentation.yaml lines 422–426 & 442–449 (GET /v1/user/list) – now
text/plain✅
• Other negative responses (e.g. POST /v1/user/create 400/409, HEAD/GET /v1/user/retrieve 4xx) – remainapplication/json
• If any of those belong toarrayResultHandler, update their docs totext/plain
No body for a successfully handled request (status code 200). |
|
Tip For best results, initiate chat on the files or code changes. @RobinTail You raise a good question about HEAD response behavior! However, according to the HTTP specification, HEAD responses should never include a response body, regardless of the status code - whether it's a successful 200 response or an error response like 400. Here's how HEAD method works:
So for a 400 error:
The headers themselves (like This behavior is consistent across all HTTP status codes - HEAD responses should always have empty bodies, but the headers should reflect what a corresponding GET request would return. |
|
Ok. Thanx. I will address it in another PR then, @coderabbitai |
|
Tip For best results, initiate chat on the files or code changes. @RobinTail That sounds like a great approach! Keeping the current PR focused on the MIME type fix and addressing the HEAD response body issue separately makes perfect sense. Would you like me to create a GitHub issue to track the HEAD response body improvement for your future PR? This would help ensure it doesn't get forgotten and provide a place to discuss the implementation details. |
) Fixes #2858 Noticed by @coderabbitai in #2857 (comment) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Updated API documentation for several HEAD endpoints to remove detailed error response content, reflecting that no payload is returned for negative responses. * **Refactor** * Simplified logic for determining response content in HEAD requests, now relying solely on the HTTP method. * Streamlined type definitions for negative responses of HEAD endpoints to indicate no content is returned. * **Tests** * Updated tests to align with the simplified response content logic for HEAD requests. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary by CodeRabbit
Bug Fixes
Documentation