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

JRuby::Rack::ErrorApp response body is ignored #194

Closed
wants to merge 1 commit into from

Conversation

yousuketto
Copy link

When env["rack.showstatus.detail"] is true, JRuby::Rack::ErrorApp response body is overwritten by JRuby::Rack::ErrorApp::ShowStatus.
https://github.com/jruby/jruby-rack/blob/1.1.18/src/main/ruby/jruby/rack/error_app/show_status.rb#L23-L31
Currently, env["rack.showstatus.detail"] will always be true.
https://github.com/jruby/jruby-rack/blob/1.1.18/src/main/ruby/jruby/rack/error_app.rb#L59

@kares
Copy link
Member

kares commented Apr 7, 2015

Hey Yousuke, I'm not really sure I follow ... we're meant to override the body (and thus not use what @app.call returned) since we're handing an error and the ErrorApp is supposed to return an errror specific body.

Some of your hints are useful and I will use them to tune the logic but the commit as is can not get in.
Try to explain your use-case and if it's no too "weird" :) we can maybe do something about it ...

@yousuketto
Copy link
Author

About this pull reqest

Response body was not overridden in v1.1.10,
but response body is overridden in v1.1.18.
Therefore, I thought it is a bug.
However, it means that specification was changed?

About my problem...

Thanks to your kind instruction, I understood my problem can be solved by editing my web.xml.
So, It is no longer a big problem.

<!-- web.xml -->
  <context-param>
    <param-name>jruby.rack.error.app</param-name>
    <param-value>require 'jruby/rack/error_app'; run JRuby::Rack::ErrorApp.new</param-value>
  </context-param>

@yousuketto yousuketto closed this Apr 23, 2015
@kares
Copy link
Member

kares commented Apr 29, 2015

good work-around, on next version I was hoping to rewrite the ErrorApp a bit (we'll see when and if that happens), for now I tuned the related code slightly so that there's more user-control ... now one can set env['rack.showstatus.detail'] = false and if the response has content and is not an error it won't "override the body" ... thanks for the report.

@yousuketto
Copy link
Author

thanks 👍

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.

2 participants