Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Always send lowlevel_error response to client (#2731)" #2809

Merged
merged 1 commit into from Jan 26, 2022

Conversation

dentarg
Copy link
Member

@dentarg dentarg commented Jan 26, 2022

This reverts commit f8acac1.

In response to #2808

@dentarg
Copy link
Member Author

dentarg commented Jan 26, 2022

I think the TruffleRuby issues are unrelated (fails on compile step): https://github.com/puma/puma/runs/4953876626?check_suite_focus=true#step:7:12

@olleolleolle
Copy link
Contributor

@dentarg The issue with truffleruby there, could that be sidestepped by a gem install bundler just for that Ruby, in that job?

@nateberkopec nateberkopec merged commit 7008a61 into puma:master Jan 26, 2022
@dentarg dentarg deleted the issue-2808 branch January 27, 2022 00:32
@dentarg
Copy link
Member Author

dentarg commented Jan 27, 2022

@olleolleolle Maybe, but I don't think we should need to do that? On the other hand, all MRI jobs have the argument setup-ruby-ref: MSP-Greg/ruby-setup-ruby/gem-update to MSP-Greg/setup-ruby-pkgs@v1 whereas non-MRI jobs don't have that

Ping @MSP-Greg

@dentarg
Copy link
Member Author

dentarg commented Jan 27, 2022

Hmm, it failed with plain ruby/setup-ruby too: https://github.com/dentarg/puma/runs/4971018457?check_suite_focus=true

/Users/runner/.rubies/truffleruby-22.0.0.2/lib/gems/gems/bundler-2.3.6/lib/bundler/runtime.rb:309:in `check_for_activated_spec!': You have already activated bundler 2.2.22, but your Gemfile requires bundler 2.3.6. Since bundler is a default gem, you can either remove your dependency on it or try updating to a newer version of bundler that supports bundler as a default gem. (Gem::LoadError)

@eregon Is this an issue in ruby/setup-ruby or with TruffleRuby?

@eregon
Copy link
Contributor

eregon commented Jan 27, 2022

Already reported as oracle/truffleruby#2586.
Seems the combination of latest bundler + truffleruby 22.0 is problematic (truffleruby-head + bundler shipped with truffleruby works, 21.3 + latest bundler works too).

baelter added a commit to baelter/puma that referenced this pull request Jan 27, 2022
casperisfine pushed a commit to casperisfine/puma that referenced this pull request Feb 11, 2022
Another fallout from puma#2809 is that
in some cases the `res_body.close` wasn't called because some previous code
raised.

For Rails apps it means CurrentAttributes and a few other important
states aren't reset properly.

This is being improved on the Rails side too, but I believe it would
be good to harden this on the puma side as well.
nateberkopec pushed a commit that referenced this pull request Feb 11, 2022
Another fallout from #2809 is that
in some cases the `res_body.close` wasn't called because some previous code
raised.

For Rails apps it means CurrentAttributes and a few other important
states aren't reset properly.

This is being improved on the Rails side too, but I believe it would
be good to harden this on the puma side as well.
nateberkopec pushed a commit that referenced this pull request Feb 11, 2022
Another fallout from #2809 is that
in some cases the `res_body.close` wasn't called because some previous code
raised.

For Rails apps it means CurrentAttributes and a few other important
states aren't reset properly.

This is being improved on the Rails side too, but I believe it would
be good to harden this on the puma side as well.
nateberkopec added a commit that referenced this pull request Feb 11, 2022
* Ensure `close` is called on the response body no matter what

Another fallout from #2809 is that
in some cases the `res_body.close` wasn't called because some previous code
raised.

For Rails apps it means CurrentAttributes and a few other important
states aren't reset properly.

This is being improved on the Rails side too, but I believe it would
be good to harden this on the puma side as well.

* 5.6.2

Co-authored-by: Jean Boussier <jean.boussier@gmail.com>
MSP-Greg pushed a commit to MSP-Greg/puma that referenced this pull request Apr 2, 2022
Another fallout from puma#2809 is that
in some cases the `res_body.close` wasn't called because some previous code
raised.

For Rails apps it means CurrentAttributes and a few other important
states aren't reset properly.

This is being improved on the Rails side too, but I believe it would
be good to harden this on the puma side as well.
MSP-Greg pushed a commit to MSP-Greg/puma that referenced this pull request Apr 3, 2022
Another fallout from puma#2809 is that
in some cases the `res_body.close` wasn't called because some previous code
raised.

For Rails apps it means CurrentAttributes and a few other important
states aren't reset properly.

This is being improved on the Rails side too, but I believe it would
be good to harden this on the puma side as well.
JuanitoFatas pushed a commit to JuanitoFatas/puma that referenced this pull request Sep 9, 2022
JuanitoFatas pushed a commit to JuanitoFatas/puma that referenced this pull request Sep 9, 2022
Another fallout from puma#2809 is that
in some cases the `res_body.close` wasn't called because some previous code
raised.

For Rails apps it means CurrentAttributes and a few other important
states aren't reset properly.

This is being improved on the Rails side too, but I believe it would
be good to harden this on the puma side as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants