Error block not working #578

Closed
kapso opened this Issue Oct 1, 2012 · 19 comments

Comments

Projects
None yet
8 participants

kapso commented Oct 1, 2012

I have the following Sinatra app and I am testing the error block but it doesnt seem to be working i.e I do not see anything in the std output.

require 'sinatra'

error do
  puts "----> Failed"
  $stdout.print "----> Failed"
end

get "/error" do
  raise "OMG"
end

I am using sinatra (1.3.3)

I found this in documentation:
"Sinatra installs special not_found and error handlers when running under the development environment."
What is you environment?

Try with:
ruby test_error.rb -env production

Member

burningTyger commented Oct 1, 2012

I checked and tested some different cases:
If you have status code error handlers like error 500 {} and you halt 500 you're fine. If you raise somewhere and you have a plain error handler nothing will happen. Same goes for the custom Errors. They will only raise an exception on a missing constant.

Contributor

blambeau commented Oct 2, 2012

I've observed a similar behavior in one of my apps recently. Error handling is generally fine except in development mode. I don't know if this is really intended, but it is not very intuitive IMHO...

@kapso then, do you fixed the problem?

Contributor

JonRowe commented Feb 24, 2013

Hi, I looked into this for you with the aim to writing a patch, the issue is that in :development mode raise_errors is set so that Sinatra can do the magic stack trace page, if you want to test the behaviour of your handlers you need to set raise_errors to be false. See the mapped error tests for more information.

Owner

rkh commented Feb 26, 2013

Error blocks don't trigger in development mode.

rkh closed this Feb 26, 2013

Contributor

blambeau commented Feb 26, 2013

@rkh that's very odd.

Owner

rkh commented Feb 26, 2013

No, it's intentional. The idea is that error blocks will hide the issue and you usually don't want to do this in development mode. Do a set :show_exceptions, :after_handler if you want to change that behaviour (see README.md).

Contributor

JonRowe commented Feb 26, 2013

Agreed it makes complete sense.

Contributor

blambeau commented Feb 26, 2013

@rkh @JonRowe only when you use exceptions passively for unintended issues IMHO. I tend to use them actively, as a programming mechanism of its own. in that context, it's odd.

EDIT: the difference between "exceptions should not occur, at least I hope so" and "exceptions will occur, handle them"

Contributor

JonRowe commented Feb 26, 2013

@blambeau That's fine, rescue them before they get to the output in sinatra, or set :raise_errors, true or set :show_exceptions, :custom_handler like @rkh suggests. Your use case isn't the standard / expected behaviour.

:)

Contributor

blambeau commented Feb 26, 2013

@JonRowe I understand that point of view. I'm not sure to agree with "Your use case isn't the expected behaviour." though.

I can imagine that most people would interpret sinatra's error blocks as a standard try/catch mechanism, mostly because there are very few other exception mechanisms available. Imagine what you would think about ruby changing its behavior wrt rescue clauses according to an environment variable. Wouldn't you think it to be (very) odd?? In web, rack/sinatra is your dev platform and it's recue clauses are odd by default ;-)

Contributor

blambeau commented Feb 26, 2013

@rkh @JonRowe that's basically why I've created Rack::Robustness (https://github.com/blambeau/rack-robustness). Seems much more usable IMHO.

Owner

rkh commented Feb 26, 2013

@rkh @JonRowe only when you use exceptions passively for unintended issues IMHO. I tend to use them actively, as a programming mechanism of its own. in that context, it's odd.

EDIT: the difference between "exceptions should not occur, at least I hope so" and "exceptions will occur, handle them"

Yeah, I wouldn't do that and generally think that's not the right approach, but if you do it that way, why not just change that one setting?

Contributor

JonRowe commented Feb 26, 2013

I still believe you are the hah, pardon the pun, exception here :) You still have try/catch you still have rescue, this is additional, designed to prevent errors escaping into the rack stack. You can turn it off if you need to.

Contributor

blambeau commented Feb 26, 2013

@JonRowe @rkh I'm the pun exception here, I'm sure (as usual I would add). And I change the setting everytime, and it's ok at first glance.

@rkh I seriously wonder what "the right approach" to exception handling really is. There are very few answers in the SE litterature and most devs simply ignore them completely. try/catch is the (only?) universal mechanism in imperative languages and sinatra departs from it without really providing a serious alternative. If you have a "right approach" to share, I'm much open to learn about it and to co-write a scientific paper with you ;-)

dariocravero referenced this issue in padrino/padrino-framework Mar 19, 2013

Merged

Failing test case for error method. #1149

Contributor

dariocravero commented Mar 19, 2013

@rkh I found this confusing too... Particularly the fact that not_found is used but error 404 blocks aren't -despite the environment you're in. We found this while adding a few custom pages for our admin in Padrino, see #1149.

Here's a sample app:

require 'sinatra'

set :show_exceptions, :after_handler

get '/hi' do
  "Hello World!"
end

get '/force' do
  halt 404
end

error 404 do
  "I'm a 404"
end

not_found do
  "I'm a not_found 404"
end

If you go to any undefined route you get "I'm a not_found 404". If you comment that block out, the "I'm a 404" block never happens which according to the logic described above that's the expected behaviour. Is there a reason why these two don't behave consistently?

Then, when I go to /force, I always get "I'm a 404" despite the fact that the not_found block is before or after the error 404 block. Is this OK?

damncabbage referenced this issue in rubyaustralia/ruby-conf-au Nov 7, 2014

Merged

Fixes HTTP 500 on file-not-found. #17

blizz commented Jan 12, 2015

The default behavior should be to use the custom error block in both production and in development. I can understand the rationale of not wanting to show stack traces in production, but this should be done without affecting the custom error block. IMHO This bothers me as well...

ujifgc referenced this issue in padrino/padrino-framework Mar 22, 2015

Closed

"error 404 do" in app/app.rb doesn't do anything #1897

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