Merge branch 'main' into nw/incompatible_login_check

.github/ISSUE_TEMPLATE/bug-report.md

@@ -6,36 +6,41 @@ labels: "Type: Bug 🐛"
 
 # Issue summary
 
-<!--
-
-Write a short description of the issue here. Please provide any details or logs that
-can help us debug it.
+Before opening this issue, I have:
+
+- [ ] Upgraded to the latest version of the package
+  - `shopify_app` version:
+  - Ruby version:
+  - Operating system:
+- [ ] Set `log_level: :debug` [in my configuration](https://github.com/Shopify/shopify-api-ruby#setup-shopify-context), if applicable
+- [ ] Found a reliable way to reproduce the problem that indicates it's a problem with the package
+- [ ] Looked for similar issues in this repository
+- [ ] Checked that this isn't an issue with a Shopify API
+  - If it is, please create a post in the [Shopify community forums](https://community.shopify.com/c/partners-and-developers/ct-p/appdev) or report it to [Shopify Partner Support](https://help.shopify.com/en/support/partners/org-select)
 
-Increase the logs as described in the README by setting log_level to :debug, and paste the relevant portion here.
-
-Learn more: https://github.com/Shopify/shopify-api-ruby#setup-shopify-context
+<!--
+Write a short description of the issue here.
 
+We can only fix issues for which there is a clear reproduction scenario.
+The more context you can provide, the easier it becomes for us to investigate and fix the issue.
 -->
 
-- `shopify_api` version:
-- `shopify_app` version:
-- Ruby version:
-- Operating system:
-
-```
-// Paste any relevant logs here
-```
-
 ## Expected behavior
 
-<!-- What do you think should happen? -->
+What do you think should happen?
 
 ## Actual behavior
 
-<!-- What actually happens? -->
+What actually happens?
 
 ## Steps to reproduce the problem
 
 1.
 1.
 1.
+
+## Debug logs
+
+```
+// Paste any relevant logs here
+```

CHANGELOG.md

@@ -4,6 +4,8 @@ Unreleased
 * ⚠️ [Breaking] Removes deprecated controller concerns that were renamed in `v21.10.0`. [1805](https://github.com/Shopify/shopify_app/pull/1805)
 * ⚠️ [Breaking] Removes deprecated `ScripttagManager`. We realize there was communication error in our logging where we logged future deprecation instead of our inteded removal. Since we have been logging that for 2 years we felt we'd move forward with the removal instead pushing this off until the next major release. [1806](https://github.com/Shopify/shopify_app/pull/1806)
 * ⚠️ [Breaking] Thows an error if a controller includes incompatible concerns (LoginProtection/EnsureInstalled) [1809](https://github.com/Shopify/shopify_app/pull/1809)
+* ⚠️ [Breaking] No longer rescues non-shopify API errors during OAuth
+  callback [1807](https://github.com/Shopify/shopify_app/pull/1807)
 * Make type param for webhooks route optional. This will fix a bug with CLI initiated webhooks.[1786](https://github.com/Shopify/shopify_app/pull/1786)
 * Fix redirecting to login when we catch a 401 response from Shopify, so that it can also handle cases where the app is already embedded when that happens.[1787](https://github.com/Shopify/shopify_app/pull/1787)
 * Always register webhooks with offline sessions.[1788](https://github.com/Shopify/shopify_app/pull/1788)

app/controllers/shopify_app/callback_controller.rb

@@ -10,8 +10,12 @@ def callback
       begin
         api_session, cookie = validated_auth_objects
       rescue => error
-        error.class.module_parent == ShopifyAPI::Errors ? callback_rescue(error) : deprecate_callback_rescue(error)
-        return respond_with_error
+        if error.class.module_parent == ShopifyAPI::Errors
+          callback_rescue(error)
+          return respond_with_error
+        else
+          raise error
+        end
       end
 
       save_session(api_session) if api_session

docs/Upgrading.md

@@ -59,6 +59,9 @@ Script tag usage has largely been replaced with the adoption of [theme app exten
 
 If you find yourself still using Scipt Tags and want to continue the pattern of declarative management of script tags this gem used to use, we recommend porting the logic [the manager used in prior versions](https://github.com/Shopify/shopify_app/blob/2336fabc6d0b45a4dee3f336455dace4d2d88bc4/lib/shopify_app/managers/scripttags_manager.rb#L4) and implementing it in a [post authentication job](https://github.com/Shopify/shopify_app/blob/main/docs/shopify_app/authentication.md#run-jobs-after-the-oauth-flow). This is the recommended flow to create script tags (or any other logic) for stores that install your app.
 
+#### No longer rescue non-shopify API errors during customized OAuth flow
+If you have customized authentication logic and are counting on the `CallbackController` to catch your error and redirect to login, you'll need to catch that error and redirect to `login_url_with_optional_shop`.
+
 ## Upgrading to 21.3.0
 The `Itp` controller concern has been removed from `LoginProtection` which is included by the `Authenticated`/`EnsureHasSession` controller concern.
 If any of your controllers are dependant on methods from `Itp` then you can include `ShopifyApp::Itp` directly.

test/controllers/callback_controller_test.rb

@@ -84,21 +84,21 @@ class CallbackControllerTest < ActionController::TestCase
         params: { shop: SHOP_DOMAIN, code: "code", state: "state", timestamp: "timestamp", host: "host", hmac: "hmac" }
     end
 
-    test "#callback rescued non-shopify errors will be deprecated" do
+    test "#callback rescued non-shopify errors are re-raised" do
       error = StandardError.new
       ShopifyAPI::Auth::Oauth.expects(:validate_auth_callback).raises(error)
 
-      message = <<~EOS
-        An error of type #{error.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
-      version = "22.0.0"
-
-      assert_within_deprecation_schedule(version)
-      ShopifyApp::Logger.expects(:deprecated).with(message, version)
-      get :callback,
-        params: { shop: SHOP_DOMAIN, code: "code", state: "state", timestamp: "timestamp", host: "host", hmac: "hmac" }
+      assert_raise StandardError do
+        get :callback,
+          params: {
+            shop: SHOP_DOMAIN,
+            code: "code",
+            state: "state",
+            timestamp: "timestamp",
+            host: "host",
+            hmac: "hmac",
+          }
+      end
     end
 
     test "#callback calls ShopifyAPI::Auth::Oauth.validate_auth_callback" do