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.

Sign up for GitHub

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

Jump to bottom

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

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

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

jkowens opened this issue Jan 2, 2018 · 3 comments
Milestone
v2.0.1

Comments

@jkowens
Copy link
Member

jkowens commented Jan 2, 2018 •
edited
Loading

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
Copy link
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 jkowens mentioned this issue Jan 5, 2018
Merged
@jkowens
Copy link
Member Author

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
Copy link
Member Author

jkowens commented Jan 5, 2018

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

@jkowens jkowens mentioned this issue Jan 6, 2018
Merged
@namusyaka namusyaka added this to the v2.0.1 milestone Feb 6, 2018
@jszwedko jszwedko mentioned this issue Jul 17, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v2.0.1
Development

No branches or pull requests
3 participants
@zzak @namusyaka @jkowens