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

NoMethodError bad_request? when Rack app mounted along with Sinatra #1376

Closed
jkowens opened this Issue Jan 2, 2018 · 3 comments

Comments

Projects
None yet
3 participants
@jkowens
Member

jkowens commented Jan 2, 2018

Sinatra is currently setting the value for constant Rack::ShowExceptions::TEMPLATE with the value Sinatra::ShowExceptions::TEMPLATE. The problem is that the new template references a method that only exists in Sinatra::ShowExceptions. When other Rack based apps like Grape are used alongside Sinatra an error is raised when attempting to display the error page.

lib/sinatra/show_exceptions.rb

...

Rack::ShowExceptions.send :remove_const, "TEMPLATE"
Rack::ShowExceptions.const_set "TEMPLATE", Sinatra::ShowExceptions::TEMPLATE

In commit 57c2ebb @kgrz mentioned an alternate solution would be override the pretty method. Maybe that would be the right path forward?

See:

ruby-grape/grape/issues/1669

@zzak

This comment has been minimized.

Show comment
Hide comment
@zzak

zzak Jan 3, 2018

Member

@jkowens Thank you for finding this!

I agree, and this should be considered a regression we should fix in 2.0.1 /cc #1339

Member

zzak commented Jan 3, 2018

@jkowens Thank you for finding this!

I agree, and this should be considered a regression we should fix in 2.0.1 /cc #1339

@jkowens

This comment has been minimized.

Show comment
Hide comment
@jkowens

jkowens Jan 5, 2018

Member

I've sent up PR rack/rack/pull/1223. Hopefully if things work out this will be an easy fix on the Sinatra side.

Member

jkowens commented Jan 5, 2018

I've sent up PR rack/rack/pull/1223. Hopefully if things work out this will be an easy fix on the Sinatra side.

@jkowens

This comment has been minimized.

Show comment
Hide comment
@jkowens

jkowens Jan 5, 2018

Member

In the meantime, I think overriding the pretty method is the best option.

Member

jkowens commented Jan 5, 2018

In the meantime, I think overriding the pretty method is the best option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment