Add support for rack.response_finished (#3681)#9
Add support for rack.response_finished (#3681)#9MitchLewis930 wants to merge 1 commit intopr_059_beforefrom
rack.response_finished (#3681)#9Conversation
* Add support for `rack.response_finished` It is very similar to `rack.after_reply`, but is part of the Rack spec. It can't just be an alias because the spec state it has to invoke callbacks in reverse order. Fix: puma#3631 Ref: rack/rack#1777 Ref: rack/rack#1802 Co-Authored-By: Robert Laurin <robert.laurin@shopify.com> * request.rb - more rename `e` to `error` changes --------- Co-authored-by: Robert Laurin <robert.laurin@shopify.com> Co-authored-by: MSP-Greg <Greg.mpls@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR adds support for the rack.response_finished callback mechanism to Puma, allowing applications to register callbacks that execute after a response is sent to the client, including when errors occur.
Changes:
- Added
RACK_RESPONSE_FINISHEDconstant and initialization in the environment - Implemented callback execution in the request handling flow, with error handling for callbacks
- Added comprehensive tests for both successful responses and error cases
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| lib/puma/const.rb | Adds the RACK_RESPONSE_FINISHED constant definition |
| lib/puma/binder.rb | Initializes RACK_AFTER_REPLY and RACK_RESPONSE_FINISHED to nil in the base environment |
| lib/puma/request.rb | Implements callback execution logic with error tracking and exception handling |
| test/test_rack_server.rb | Adds tests for response_finished callbacks in both success and error scenarios |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @simple.call(env) | ||
| end | ||
|
|
||
|
|
There was a problem hiding this comment.
Extra blank line should be removed to maintain consistent spacing in the test file.
| end | ||
| end | ||
|
|
||
| if response_finished = env[RACK_RESPONSE_FINISHED] |
There was a problem hiding this comment.
Using assignment within a conditional expression can be confusing. Consider using if (response_finished = env[RACK_RESPONSE_FINISHED]) with explicit parentheses to clarify intent, or extract the assignment to a separate line for better readability.
| if response_finished = env[RACK_RESPONSE_FINISHED] | |
| response_finished = env[RACK_RESPONSE_FINISHED] | |
| if response_finished |
PR_059