Skip to content

Content type is forced to text/html in non-test mode #500

Closed
myronmarston opened this Issue Apr 5, 2012 · 7 comments

5 participants

@myronmarston

I have a super simple app that serves up json. I'm using the before and not_found hooks to set the content type and response bodies for 404s:

before    { content_type "application/json;charset=utf-8" }
not_found { JSON.dump("error" => "Not Found") }

I wrote a test for this with rack-test asserting the content type was "application/json" and it worked. Then I booted the app and hit an undefined endpoint and got a content-type of "text/html". After digging into the problem, it appears that the value of RACK_ENV affects this. In my test environment, it was set to test. When I booted my app it defaulted to development.

It seems wrong that sinatra would honor the content type I have explicitly set in the test environment but not in other environments. It makes it hard to trust your tests when it behaves differently in that environment :(.

I put together a gist demonstrating the issue:

https://gist.github.com/2313727

Notice that the test passes when RACK_ENV=test is set but fails otherwise.

@rkh rkh was assigned Apr 10, 2012
@konieczkow

Problem occurs only in developer mode (it works as expected when RACK_ENV='production'). It is caused by the fact that in developer mode, errors are handled differently.

The problem is deeper, and what you see is just the tip of the iceberg. At this time, it works like this (with RACK_ENV='development':

  • first content of 'before' block is evaluated,
  • then configure :development sets content_type to 'text/html' (line: 1602)
configure :development do
  get '/__sinatra__/:image.png' do
    filename = File.dirname(__FILE__) + "/images/#{params[:image]}.png"
    content_type :png
    send_file filename
  end

  error NotFound do
    content_type 'text/html'

    (<<-HTML).gsub(/^ {8}/, '')
    <!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&rsquo;t know this ditty.</h2>
      <img src='#{uri "/__sinatra__/404.png"}'>
      <div id="c">
        Try this:
        <pre>#{request.request_method.downcase} '#{request.path_info}' do\n  "Hello World"\nend</pre>
      </div>
    </body>
    </html>
    HTML
  end
end
  • then not_found block is evaluated As you can see, this sequence causes the settings from 'before' they are overwritten by the contents of the block configure

I have no idea at this time how to correct this error, I hope that the additional information I have given will be helpful.

@civrot
civrot commented Nov 16, 2012

Is this really bug? There has been no activity for 7 months. Should this bug be closed?

@myronmarston

@civrot, I still consider it a bug.

@rkh
Sinatra member
rkh commented Dec 18, 2012

This is a bug, sorry for the lack of activity.

@rkh
Sinatra member
rkh commented Dec 18, 2012

If someone would provide a test for this, it would extremely speed up things.

@myronmarston
@jtarchie

Just because I haven't figured out all the fancy markdown links github does.

jtarchie/sinatra@91a587a

This is a failing test of the problem occurring. As @konieczkow said, it appears to be happening because the error NotFound defined for the development mode is happening before the one defined by the user.

@rkh rkh added a commit that referenced this issue Feb 26, 2013
@rkh rkh added test for #500 a51beb0
@rkh rkh closed this in c9c0e55 Feb 26, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.