search

jruby / jruby-rack

Rack for JRuby and Java appservers
MIT License
397 stars 137 forks source link

NoMethodError undefined method get_header re-emerged with JRuby Rack 1.2.2 #259

Open chadlwilson opened 2 months ago

chadlwilson commented 2 months ago

jruby-rack 1.2.x seems to barf while running on app that was previously working fine with

NoMethodError (undefined method `get_header' for #<Rack::Handler::Servlet::DefaultEnv:0xfb5b374>):

This seems to be a re-emergence of #211, previously fixed by #212 in 1.1.21 to create Rails 5.x compatibility, but merged only to the 1.1 branch. Not sure if intentional, but might need some cherry-picks back to master for this and possibly others? https://github.com/jruby/jruby-rack/compare/1.1-stable

Happy to try and help where I can if so. I would like to see how to move forward on master to figure out #244 and others so having some working CI of some sort from master helps.

headius commented 2 months ago

previously fixed by https://github.com/jruby/jruby-rack/pull/212 in 1.2.21

I assume you mean 1.1.21?

Could you put together a new PR based on #212 that applies it to master? Alternatively we could review the "big merge" I attempted in #253 and pull in everything that was not merged.

headius commented 2 months ago

We can also release a 1.1.x release now that the fix for #247 has been fixed there as well, if we want a smaller target for the moment.

chadlwilson commented 2 months ago

previously fixed by #212 in 1.2.21

I assume you mean 1.1.21?

Ahh yes, sorry. Too many version numbers 😔

chadlwilson commented 2 months ago

We can also release a 1.1.x release now that the fix for #247 has been fixed there as well, if we want a smaller target for the moment.

Personally I'm not too worried about that particular fix, so am neutral on releasing that branch unless the intent was to also get that in a CIed state of some sort 😅

Was more looking to move forward with a branch that seems to be receiving some love, and was making the naive assumption that the intent of releasing 1.2.x was an attempt to move forward with this branch as primary - without noticing your earlier #253 PR which I guess implicitly acknowledges that there are likely a lot of regressions on 1.2/master in comparison to 1.1 until the mega merge is done.

If I PR the #212 fix I suspect we'll end up playing whack-a-mole for various issues long since fixed on 1.1 folks have been relying on for years?

headius commented 2 months ago

I don't think it would take much effort to get another 1.1.x release out with the #247 fix, but I think both @kares and I would at least like to toward 1.2 being the release path going forward. We will discuss and see what makes the most sense right now.