GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Add `rack.response_finished` to `Rack::Lint`. by ioquatix · Pull Request #1952 · rack/rack

Conversation

@ioquatix

@ioquatix ioquatix commented Aug 26, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

This updates Rack::Lint to validate that rack.response_finished is an
array of callables when present in the env. e.g. procs, lambdas, or
objects that respond to call.

This validates that:

  • rack.response_finished is an array
  • The contents of the array all respond to call

And specifies that:

  • The callback will receive (env, status, headers, error) as arguments.

Bringing #1802 to completion according to @tenderlove's recommendations/requirements.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@ioquatix ioquatix changed the title Add rack.response_finished to Rack::Lint Add rack.response_finished to Rack::Lint. Aug 26, 2022
@ioquatix
ioquatix force-pushed the rack-response-finished branch from 137f5a8 to aa81c86 Compare August 26, 2022 06:06
@ioquatix
ioquatix requested review from jeremyevans and tenderlove August 26, 2022 06:07
@ioquatix
ioquatix force-pushed the rack-response-finished branch 4 times, most recently from 6672eb2 to 9c88c6a Compare August 26, 2022 07:39
@BlakeWilliams @ioquatix
Add rack.response_finished to Rack::Lint
d804915 
This updates Rack::Lint to validate that `rack.response_finished` is an
array of callables when present in the `env`. e.g. procs, lambdas, or
objects that respond to `call`.

This validates that:

* `rack.response_finished` is an array
* The contents of the array all respond to `call`
@ioquatix
ioquatix force-pushed the rack-response-finished branch from 9c88c6a to d804915 Compare August 26, 2022 07:54
@ioquatix ioquatix added the SPEC label Aug 26, 2022
jeremyevans
jeremyevans reviewed Aug 26, 2022
View reviewed changes

@jeremyevans jeremyevans left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't check that if rack.response_finished is present, each entry in the array, if called, is called with the four arguments. I'm not sure whether it is not worth wrapping each callable in the array in a wrapper that does such checks, but we do take similar steps in other cases (e.g. InputWrapper, ErrorWrapper, StreamWrapper).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
ioquatix and Fulya2133 reacted with thumbs up emoji Fulya2133 reacted with rocket emoji
All reactions
  • 👍 2 reactions
  • 🚀 1 reaction
Comment thread SPEC.rdoc Resolved

Uh oh!

There was an error while loading. Please reload this page.
@ioquatix
Clarified more behaviour.
4098c4b
@ioquatix
ioquatix requested a review from jeremyevans August 26, 2022 22:17
tenderlove
tenderlove approved these changes Aug 26, 2022
View reviewed changes
@ioquatix
ioquatix merged commit 856c4f9 into main Aug 26, 2022
@ioquatix
ioquatix deleted the rack-response-finished branch August 26, 2022 22:25
This was referenced Aug 26, 2022
Add rack.response_finished to Rack::Lint #1802
Closed
Can rack.after_reply be added to the spec? #1777
Closed
@martinrehfeld martinrehfeld mentioned this pull request Jan 19, 2023
Bump rack from 2.2.4 to 2.2.6.2 martinrehfeld/gufm1-webservice#8
Merged
@ckraybill ckraybill mentioned this pull request Jan 23, 2024
rack.after_reply phusion/passenger#2418
Closed
@casperisfine casperisfine mentioned this pull request Mar 8, 2024
Improve rack.after_reply Shopify/pitchfork#97
Closed
casperisfine pushed a commit to Shopify/pitchfork that referenced this pull request Mar 11, 2024
@byroot
Implement rack.response_finished
7ac65bb 
Fix: #97
Ref: rack/rack#1952

Similar to `rack.after_reply` but this one is part of the Rack
spec and expect some arguments.
@MSP-Greg MSP-Greg mentioned this pull request Jun 20, 2025
Add suport for rack.response_finished puma/puma#3631
Closed
5 of 7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tenderlove tenderlove tenderlove approved these changes
@jeremyevans jeremyevans Awaiting requested review from jeremyevans

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

SPEC

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@ioquatix @tenderlove @jeremyevans @BlakeWilliams
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.