Skip to content

Commit

Permalink
Merge pull request #1035 from omniauth/1030-standard-error-handling
Browse files Browse the repository at this point in the history
Better handle errors that come from the actual app.
  • Loading branch information
BobbyMcWho committed Feb 18, 2021
2 parents 0d533c3 + 6f4cdb0 commit 7e1b49f
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 0 deletions.
7 changes: 7 additions & 0 deletions lib/omniauth/strategy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,8 @@ def call!(env) # rubocop:disable CyclomaticComplexity, PerceivedComplexity
return callback_call if on_callback_path?
return other_phase if respond_to?(:other_phase)
rescue StandardError => e
raise e if env.delete('omniauth.error.app')

return fail!(e.message, e)
end

Expand Down Expand Up @@ -302,6 +304,8 @@ def mock_call!(*)
return mock_request_call if on_request_path? && OmniAuth.config.allowed_request_methods.include?(request.request_method.downcase.to_sym)
return mock_callback_call if on_callback_path?
rescue StandardError => e
raise e if env.delete('omniauth.error.app')

return fail!(e.message, e)
end

Expand Down Expand Up @@ -462,6 +466,9 @@ def query_string

def call_app!(env = @env)
@app.call(env)
rescue StandardError => e
env['omniauth.error.app'] = true
raise e
end

def full_host
Expand Down
30 changes: 30 additions & 0 deletions spec/omniauth/strategy_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,36 @@ def make_env(path = '/auth/test', props = {})
expect(strategy.callback_url).to eq('http://example.com/myapp/override/callback')
end
end

context 'error during call_app!' do
class OtherPhaseStrategy < ExampleStrategy
def other_phase
call_app!
end
end
class AppError < StandardError; end
let(:app) { ->(_env) { raise(AppError.new('Some error in the app!')) } }
let(:strategy) { OtherPhaseStrategy.new(app) }

it 'raises the application error' do
expect { strategy.call(make_env('/some/path')) }.to raise_error(AppError, 'Some error in the app!')
end
end

context 'error during auth phase' do
class SomeStrategy < ExampleStrategy
def request_phase
raise AuthError.new('Some error in authentication')
end
end
class AuthError < StandardError; end
let(:strategy) { SomeStrategy.new(app) }

it 'passes the error to fail!()' do
expect(strategy).to receive(:fail!).with('Some error in authentication', kind_of(AuthError))
strategy.call(make_env('/auth/test', props))
end
end
end


Expand Down

0 comments on commit 7e1b49f

Please sign in to comment.