From 6f4cdb08f4741a0b3c83dfafd5f93e972be97c0e Mon Sep 17 00:00:00 2001
From: Bobby McDonald <bobbymcwho@gmail.com>
Date: Fri, 12 Feb 2021 12:17:04 -0500
Subject: [PATCH] Better handle errors that come from the actual app.

As a consequence of the changes that were merged in #689, errors
thrown by strategies that utilize other_phase (or more specifically
call_app!), would be caught by omniauth, causing headaches for folks
looking to have those errors handled by their application. This
should allow for errors that come from the app to pass through, while
passing errors that come from the authentication phases to the fail!
handler.
---
 lib/omniauth/strategy.rb       |  7 +++++++
 spec/omniauth/strategy_spec.rb | 30 ++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/lib/omniauth/strategy.rb b/lib/omniauth/strategy.rb
index 2d5018a..a2590ce 100644
--- a/lib/omniauth/strategy.rb
+++ b/lib/omniauth/strategy.rb
@@ -193,6 +193,8 @@ module OmniAuth
         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
 
@@ -302,6 +304,8 @@ module OmniAuth
         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
 
@@ -462,6 +466,9 @@ module OmniAuth
 
     def call_app!(env = @env)
       @app.call(env)
+    rescue StandardError => e
+      env['omniauth.error.app'] = true
+      raise e
     end
 
     def full_host
diff --git a/spec/omniauth/strategy_spec.rb b/spec/omniauth/strategy_spec.rb
index 89b96be..e037aad 100644
--- a/spec/omniauth/strategy_spec.rb
+++ b/spec/omniauth/strategy_spec.rb
@@ -629,6 +629,36 @@ describe OmniAuth::Strategy do
           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