Skip to content

Improve Error Handling #578

mbleigh opened this Issue Feb 21, 2012 · 3 comments

2 participants

INTRIDEA Inc. member
mbleigh commented Feb 21, 2012

Right now OmniAuth is a bit too much of a rabbit hole when it comes to error handling. I propose that the default error strategy be updated to be a little more robust:

  1. Create a new configuration option raise_exceptions that accepts a boolean value or a lambda. By default, it should be a lambda that evaluates to false unless RACK_ENV is development.
  2. Alter the default failure endpoint to raise the exception stored in omniauth.error if it exists, or raise an OmniAuth::Error with as much info as possible only if raise_exceptions is true. In environments not development, no change to the strategy will take place.

There exists an option called on_failure that gets called when a strategy calls fail!. By default, it computes some values and then returns the array that Rack expects middleware to return.

In an app at work, re-assigned OmniAuth.config.on_failure to be a block that logs, notified Airbrake and then does what the default does. It would be really nice to be able to somehow easily call super if you were wanting to prepend functionality like that, rather than having to copy the code out (which I did).

It would also be nice if OmniAuth did some logging and let you pass in an object to log to. Or maybe in some other way allow better introspection. If a strategy encounters an exception that it doesn't handle (with rescue and fail!), it can be really hard to figure out what the state of the strategy was that caused the failure. How would folks feel about an OmniAuth.log method that would log if provided with a logging object, but no-op if it hadn't?

INTRIDEA Inc. member
mbleigh commented Mar 1, 2012

Agreed. In OmniAuth 1.1 I will probably introduce something like OmniAuth::FailureEndpoint or something similar that will be a simple Rack endpoint that is the default fail strategy.


Am I correct in understanding that this would mean that other failure strategies could do whatever they were doing and then call off to OmniAuth::FailureEndpoint? If so, that sounds pretty awesome.

@mbleigh mbleigh pushed a commit that referenced this issue Mar 6, 2012
Michael Bleigh Extract default on_failure into class. References #578. 5fbc419
@mbleigh mbleigh pushed a commit that closed this issue Mar 6, 2012
Michael Bleigh Improves error handling behavior. Closes #578.
* OmniAuth::FailureEndpoint will raise any encountered
  errors out if RACK_ENV is development.
* It will redirect compatibly with current OmniAuth if
  not in development.
* Now uses Rack::Response to include a content length
  in the failure response.
@mbleigh mbleigh closed this in d02f9d5 Mar 6, 2012
@yb66 yb66 added a commit that referenced this issue Nov 2, 2012
@yb66 yb66 Added raise_exception option
Mentioned [here](d02f9d5#L3R2) and in [ticket #578](#578) but was obviously never introduced along with the other changes made.
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.