Skip to content

Commit

Permalink
ShowException only serves HTML Accept header contains text/html
Browse files Browse the repository at this point in the history
Rather than be concerned with whether a request is an asynchronous browser
request or not it is better to simply consider the Accept header and only serve
HTML to clients that specifically ask for it.

This way you will not find your pure JSON API application splitting out HTML
error messages to your console when using curl :)
  • Loading branch information
bestie authored and raggi committed Jul 13, 2014
1 parent 71c6911 commit 2ab24cf
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 12 deletions.
12 changes: 6 additions & 6 deletions lib/rack/showexceptions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@ def call(env)
env["rack.errors"].puts(exception_string)
env["rack.errors"].flush

if prefers_plain_text?(env)
content_type = "text/plain"
body = [exception_string]
else
if accepts_html?(env)
content_type = "text/html"
body = pretty(env, e)
else
content_type = "text/plain"
body = [exception_string]
end

[500,
Expand All @@ -42,8 +42,8 @@ def call(env)
body]
end

def prefers_plain_text?(env)
env["HTTP_X_REQUESTED_WITH"] == "XMLHttpRequest" && (!env["HTTP_ACCEPT"] || !env["HTTP_ACCEPT"].include?("text/html"))
def accepts_html?(env)
env["HTTP_ACCEPT"] && env["HTTP_ACCEPT"].include?("text/html")
end

def dump_exception(exception)
Expand Down
12 changes: 6 additions & 6 deletions test/spec_showexceptions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def show_exceptions(app)
))

lambda{
res = req.get("/")
res = req.get("/", "HTTP_ACCEPT" => "text/html")
}.should.not.raise

res.should.be.a.server_error
Expand All @@ -26,7 +26,7 @@ def show_exceptions(app)
res.should =~ /ShowExceptions/
end

it "responds with plain text on AJAX requests accepting anything but HTML" do
it "responds with plain text to requests not specifically accepting HTML" do
res = nil

req = Rack::MockRequest.new(
Expand All @@ -35,7 +35,7 @@ def show_exceptions(app)
))

lambda{
res = req.get("/", "HTTP_X_REQUESTED_WITH" => "XMLHttpRequest")
res = req.get( "/", "HTTP_ACCECPT" => "*/*")
}.should.not.raise

res.should.be.a.server_error
Expand All @@ -47,7 +47,7 @@ def show_exceptions(app)
res.body.should.include __FILE__
end

it "responds with HTML on AJAX requests accepting HTML" do
it "responds with HTML to requests specifically accepting HTML" do
res = nil

req = Rack::MockRequest.new(
Expand All @@ -56,7 +56,7 @@ def show_exceptions(app)
))

lambda{
res = req.get("/", "HTTP_X_REQUESTED_WITH" => "XMLHttpRequest", "HTTP_ACCEPT" => "text/html")
res = req.get("/", "HTTP_ACCEPT" => "text/html")
}.should.not.raise

res.should.be.a.server_error
Expand All @@ -79,7 +79,7 @@ def show_exceptions(app)
)

lambda{
res = req.get("/")
res = req.get("/", "HTTP_ACCEPT" => "text/html")
}.should.not.raise

res.should.be.a.server_error
Expand Down

0 comments on commit 2ab24cf

Please sign in to comment.