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

error! method returns wrong content type "text/html" for 404 status code #1515

Closed
khelll opened this issue Nov 1, 2016 · 9 comments · Fixed by #2089
Closed

error! method returns wrong content type "text/html" for 404 status code #1515

khelll opened this issue Nov 1, 2016 · 9 comments · Fixed by #2089
Labels
bug? you can help

Comments

@khelll
Copy link

khelll commented Nov 1, 2016

When I do:

error!({message: 'no data'}, 404) or even simple one error!({}, 404)

I always get text/html for content-type with the response:

"<!DOCTYPE html>\n<html>\n<head>\n <style type=\"text/css\">\n body { text-align:center;font-family:helvetica,arial;font-size:22px;\n color:#888;margin:20px}\n #c {margin:0 auto;width:500px;text-align:left}\n </style>\n</head>\n<body>\n <h2>Sinatra doesn&rsquo;t know this ditty.</h2>\n <img src='http://localhost:7777/__sinatra__/404.png'>\n <div id=\"c\">\n Try this:\n <pre>get '/api/customers/111/info' do\n \"Hello World\"\nend\n</pre>\n </div>\n</body>\n</html>\n"

However, if I change to any other status code like error!({}, 500), it returns the expected JSON response.
@dblock
Copy link
Member

dblock commented Nov 1, 2016

Looking at the HTML I think this is caught by Sinatra. I would dig there next. Maybe try to build a quick repro? We could also use something similar on top of sinatra to https://github.com/ruby-grape/grape-on-rack and https://github.com/ruby-grape/grape-on-rails, so if you make a basic grape-on-sinatra implementation we can move it into the ruby-grape org and use it as a place to debug similar things. Maybe based on https://github.com/katgironpe/sinatra-pg-grape (without a database).

@nbulaj
Copy link
Contributor

nbulaj commented Nov 24, 2016

Hi @khelll. It is definitely the problem of Sinatra gem, not Grape. So take a look one again at your routes and rescuers. Without some code gist it is hard to understand what is going on.

@dblock
Copy link
Member

dblock commented Feb 1, 2017

Closing this, a repro would be a start if you need help @khelll.

@dblock dblock closed this as completed Feb 1, 2017
@jonmchan
Copy link
Contributor

jonmchan commented Jul 5, 2020

I ran into this today... It indeed is a setup issue. It is highly dependent on how you set up integrating grape and sinatra. For myself, I was able to fix this by changing the order of loading grape and sinatra in config.ru:

From:

run Rack::Cascade.new [API::Base, SinatraApp]

to:

run Rack::Cascade.new [SinatraApp, API::Base]

This caused the 404 to be intercepted by sinatra but be the last and final output in the middleware chain. Hope this helps someone else in this situation if you're using the same rack setup as mine!

@dblock
Copy link
Member

dblock commented Jul 6, 2020

Want to write an integration test for this @jonmchan?

@jonmchan
Copy link
Contributor

jonmchan commented Jul 7, 2020

@dblock it isn't a sinatra or a grape issue per se... this is a configuration issue. If you set sinatra downstream to grape, Sinatra will pick up all 404s and answer them. I think it is sufficient enough to leave this issue documented and the configuration instructions to solve it on this page.

@dblock
Copy link
Member

dblock commented Jul 7, 2020

Do we need a doc update for https://github.com/ruby-grape/grape#alongside-sinatra-or-other-frameworks?

@jonmchan
Copy link
Contributor

jonmchan commented Jul 7, 2020

@dblock yes - with the exact example in the doc, 404 would not be properly raised and caught by sinatra. You need change run Rack::Cascade.new [API, Web] to run Rack::Cascade.new [Web, API].

Here's example code:

# Example config.ru

require 'sinatra'
require 'grape'

class API < Grape::API
  get :hello do
    { hello: 'world' }
  end
  get '/test_404' do
    error!({error: "message"}, 404)
  end
end

class Web < Sinatra::Base
  get '/' do
    'Hello world.'
  end
end

use Rack::Session::Cookie
#run Rack::Cascade.new [API, Web]
run Rack::Cascade.new [Web, API]

With API first:

$ curl localhost:9292/test_404
::1 - - [07/Jul/2020:12:48:07 -0500] "GET /test_404 HTTP/1.1" 404 501 0.0423
<!DOCTYPE html>
<html>
<head>
  <style type="text/css">
  body { text-align:center;font-family:helvetica,arial;font-size:22px;
    color:#888;margin:20px}
  #c {margin:0 auto;width:500px;text-align:left}
  </style>
</head>
<body>
  <h2>Sinatra doesn’t know this ditty.</h2>
  <img src='http://localhost:9292/__sinatra__/404.png'>
  <div id="c">
    Try this:
    <pre># in config.ru
class Web
  get &#x27;&#x2F;test_404&#x27; do
    &quot;Hello World&quot;
  end
end
</pre>
  </div>
</body>
</html>

With Web first:

$ curl localhost:9292/test_404
::1 - - [07/Jul/2020:12:48:45 -0500] "GET /test_404 HTTP/1.1" 404 19 0.0422
{"error":"message"}

@dblock
Copy link
Member

dblock commented Jul 7, 2020

Appreciate if you could PR this @jonmchan

@jonmchan jonmchan mentioned this issue Jul 11, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug? you can help
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Update README to fix 404 error issue mounting Grape Using Rack::Cascade jonmchan/grape
4 participants
@khelll @dblock @nbulaj @jonmchan