Create fresh response object in default Fastify handler#612
Conversation
2a110f9 to
c674aef
Compare
Codecov Report✅ All modified and coverable lines are covered by tests.
🚀 New features to boost your workflow:
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical bug in the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request fixes a bug in the @fedify/fastify package where a shared Response object for 406 Not Acceptable errors was causing a 'body already consumed' error on subsequent requests. The fix refactors the default onNotAcceptable handler to create a new Response object for each request. A regression test is included to cover this scenario, and the changelog has been updated to reflect the change. The implementation correctly resolves the issue.
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
Pull request overview
Fixes a Fastify integration bug where the default onNotAcceptable handler reused a single Response instance, causing subsequent requests to fail due to a consumed response body stream.
Changes:
- Replaced the shared default 406
Responsewith a factory function to create a freshResponseper request. - Added a regression test that reproduces the “Response.body is already consumed” failure across multiple requests.
- Added a changelog entry under the upcoming release.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/fastify/src/index.ts | Switches default onNotAcceptable from a singleton Response to a per-request factory. |
| packages/fastify/src/index.test.ts | Adds regression coverage ensuring repeated Not Acceptable requests return 406 consistently. |
| CHANGES.md | Documents the fix for @fedify/fastify in v2.0.4 notes. |
c674aef to
3c4a29e
Compare
Summary
This pull request just let
@fedify/fastifycreate a freshResponseobject by default not acceptable handler.When running the
examples/fastifyproject and executing the followingcurlcommand twice:It returns 500 error because the response stream is already consumed. You can re-check it by running the regression test (in
index.test.tsfile)This pull request just make it return a new
Responseobject for each request.Related issue
There is no related issue.
Changes
List the specific modifications made in this PR.
Focus on what was changed without going into detail about impact.
Responseobject.Benefits
It will make server applications using
@fedify/fastifywithout theironNotAcceptablehandler return a valid 406 response (Not Acceptable).Checklist
mise teston your machine?