diff --git a/CHANGELOG.md b/CHANGELOG.md index 467517bc8..5466c7b7c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ Unreleased ---------- * Fixes a bug with `EnsureAuthenticatedLinks` causing deep links to not work [#1549](https://github.com/Shopify/shopify_app/pull/1549) * Ensure online token is properly used when using `current_shopify_session` [#1566](https://github.com/Shopify/shopify_app/pull/1566) +* Emit a deprecation notice for wrongly-rescued exceptions [#1530](https://github.com/Shopify/shopify_app/pull/1530) * Log a deprecation warning for the use of incompatible controller concerns [#1560](https://github.com/Shopify/shopify_app/pull/1560) 21.2.0 (Oct 25, 2022) diff --git a/app/controllers/shopify_app/callback_controller.rb b/app/controllers/shopify_app/callback_controller.rb index b27e2f6b9..5934d65e0 100644 --- a/app/controllers/shopify_app/callback_controller.rb +++ b/app/controllers/shopify_app/callback_controller.rb @@ -17,7 +17,14 @@ def callback }, auth_query: ShopifyAPI::Auth::Oauth::AuthQuery.new(**filtered_params), ) - rescue + rescue => e + unless e.class.module_parent == ShopifyAPI::Errors + ActiveSupport::Deprecation.warn(<<~EOS) + An error of type #{e.class} was rescued. This is not part of `ShopifyAPI::Errors`, which could indicate a + bug in your app, or a bug in the shopify_app gem. Future versions of the gem may re-raise this error rather + than rescuing it. + EOS + end return respond_with_error end diff --git a/test/controllers/callback_controller_test.rb b/test/controllers/callback_controller_test.rb index 44f54c50f..21e70ef9c 100644 --- a/test/controllers/callback_controller_test.rb +++ b/test/controllers/callback_controller_test.rb @@ -35,16 +35,36 @@ class CallbackControllerTest < ActionController::TestCase end test "#callback flashes error when omniauth is not present" do - get :callback, params: { shop: "shop" } + get :callback, + params: { shop: "shop", code: "code", state: "state", timestamp: "timestamp", host: "host", hmac: "hmac" } assert_equal flash[:error], "Could not log in to Shopify store" end test "#callback flashes error in Spanish" do I18n.locale = :es - get :callback, params: { shop: "shop" } + get :callback, + params: { shop: "shop", code: "code", state: "state", timestamp: "timestamp", host: "host", hmac: "hmac" } assert_match "sesiĆ³n", flash[:error] end + test "#callback rescued errors of ShopifyAPI::Error will not emit a deprecation notice" do + ShopifyAPI::Auth::Oauth.expects(:validate_auth_callback).raises(ShopifyAPI::Errors::MissingRequiredArgumentError) + assert_not_deprecated do + get :callback, + params: { shop: "shop", code: "code", state: "state", timestamp: "timestamp", host: "host", hmac: "hmac" } + end + assert_equal flash[:error], "Could not log in to Shopify store" + end + + test "#callback rescued errors other than ShopifyAPI::Error will emit a deprecation notice" do + ShopifyAPI::Auth::Oauth.expects(:validate_auth_callback).raises(StandardError) + assert_deprecated(/An error of type StandardError was rescued/) do + get :callback, + params: { shop: "shop", code: "code", state: "state", timestamp: "timestamp", host: "host", hmac: "hmac" } + end + assert_equal flash[:error], "Could not log in to Shopify store" + end + test "#callback calls ShopifyAPI::Auth::Oauth.validate_auth_callback" do mock_oauth