Merge pull request #588 from Shopify/handle_webhook_errors

Handle Webhook subscription errors after Token Exchange
Shopify · Jan 18, 2024 · 9411538 · 9411538
2 parents 7d90dcc + f445164
commit 9411538
Show file tree

Hide file tree

Showing 22 changed files with 216 additions and 43 deletions.
diff --git a/.changeset/famous-knives-fix.md b/.changeset/famous-knives-fix.md
@@ -0,0 +1,17 @@
+---
+'@shopify/shopify-app-session-storage-postgresql': patch
+'@shopify/shopify-app-session-storage-test-utils': patch
+'@shopify/shopify-app-session-storage-dynamodb': patch
+'@shopify/shopify-app-session-storage-mongodb': patch
+'@shopify/shopify-app-session-storage-memory': patch
+'@shopify/shopify-app-session-storage-prisma': patch
+'@shopify/shopify-app-session-storage-sqlite': patch
+'@shopify/shopify-app-session-storage-mysql': patch
+'@shopify/shopify-app-session-storage-redis': patch
+'@shopify/shopify-app-session-storage-kv': patch
+'@shopify/shopify-app-session-storage': patch
+'@shopify/shopify-app-express': patch
+'@shopify/shopify-app-remix': patch
+---
+
+Bump shopify-api version from 9.0.1 to 9.0.2
diff --git a/.changeset/smart-windows-smash.md b/.changeset/smart-windows-smash.md
@@ -0,0 +1,5 @@
+---
+'@shopify/shopify-app-remix': patch
+---
+
+Handle webhook registration throttling error
diff --git a/.changeset/wild-bottles-explode.md b/.changeset/wild-bottles-explode.md
@@ -0,0 +1,5 @@
+---
+'@shopify/shopify-app-remix': patch
+---
+
+Use 'body' field from GraphqlQueryError when logging session validation error
diff --git a/packages/shopify-app-express/package.json b/packages/shopify-app-express/package.json
@@ -30,7 +30,7 @@
     "Storefront API"
   ],
   "dependencies": {
-    "@shopify/shopify-api": "^9.0.1",
+    "@shopify/shopify-api": "^9.0.2",
     "@shopify/shopify-app-session-storage": "^2.0.3",
     "@shopify/shopify-app-session-storage-memory": "^2.0.3",
     "cookie-parser": "^1.4.6",

diff --git a/packages/shopify-app-remix/package.json b/packages/shopify-app-remix/package.json
@@ -69,7 +69,7 @@
   "dependencies": {
     "@remix-run/server-runtime": "^2.0.0",
     "@shopify/admin-api-client": "^0.2.1",
-    "@shopify/shopify-api": "^9.0.1",
+    "@shopify/shopify-api": "^9.0.2",
     "@shopify/shopify-app-session-storage": "^2.0.3",
     "@shopify/storefront-api-client": "^0.2.1",
     "isbot": "^3.7.1",

diff --git a/packages/shopify-app-remix/src/server/authenticate/admin/strategies/auth-code-flow.ts b/packages/shopify-app-remix/src/server/authenticate/admin/strategies/auth-code-flow.ts
@@ -306,7 +306,7 @@ export class AuthCodeFlowStrategy<
     } else if (error instanceof GraphqlQueryError) {
       const context: {[key: string]: string} = {shop};
       if (error.response) {
-        context.response = JSON.stringify(error.response);
+        context.response = JSON.stringify(error.body);
       }
 
       logger.error(

diff --git a/packages/shopify-app-remix/src/server/authenticate/webhooks/__tests__/mock-responses.ts b/packages/shopify-app-remix/src/server/authenticate/webhooks/__tests__/mock-responses.ts
@@ -32,3 +32,14 @@ export const HTTP_WEBHOOK_CREATE_ERROR_RESPONSE = {
     },
   },
 };
+
+export const HTTP_WEBHOOK_THROTTLING_ERROR_RESPONSE = {
+  errors: [
+    {
+      message: 'Throttled',
+      extensions: {
+        code: 'THROTTLED',
+      },
+    },
+  ],
+};
diff --git a/packages/shopify-app-remix/src/server/authenticate/webhooks/__tests__/register.test.ts b/packages/shopify-app-remix/src/server/authenticate/webhooks/__tests__/register.test.ts
@@ -1,4 +1,4 @@
-import {DeliveryMethod, Session} from '@shopify/shopify-api';
+import {DeliveryMethod, GraphqlQueryError, Session} from '@shopify/shopify-api';
 
 import {shopifyApp} from '../../..';
 import {
@@ -110,4 +110,101 @@ describe('Webhook registration', () => {
       NOT_A_VALID_TOPIC: [expect.objectContaining({success: false})],
     });
   });
+
+  // eslint-disable-next-line no-warning-comments
+  // TODO: Remove tests once we have a better solution to parallel afterAuth calls
+  it('logs throttling errors', async () => {
+    // GIVEN
+    const shopify = shopifyApp(
+      testConfig({
+        webhooks: {
+          NOT_A_VALID_TOPIC: {
+            deliveryMethod: DeliveryMethod.Http,
+            callbackUrl: '/webhooks',
+          },
+        },
+      }),
+    );
+    const session = new Session({
+      id: `offline_${TEST_SHOP}`,
+      shop: TEST_SHOP,
+      isOnline: false,
+      state: 'test',
+      accessToken: 'totally_real_token',
+    });
+
+    await mockExternalRequests(
+      {
+        request: new Request(GRAPHQL_URL, {
+          method: 'POST',
+          body: 'webhookSubscriptions',
+        }),
+        response: new Response(
+          JSON.stringify(mockResponses.EMPTY_WEBHOOK_RESPONSE),
+        ),
+      },
+      {
+        request: new Request(GRAPHQL_URL, {
+          method: 'POST',
+          body: 'webhookSubscriptionCreate',
+        }),
+        response: new Response(
+          JSON.stringify(mockResponses.HTTP_WEBHOOK_THROTTLING_ERROR_RESPONSE),
+        ),
+      },
+    );
+
+    // WHEN
+    const results = await shopify.registerWebhooks({session});
+
+    // THEN
+    expect(results).toBeUndefined();
+  });
+
+  it('throws other errors', async () => {
+    // GIVEN
+    const shopify = shopifyApp(
+      testConfig({
+        webhooks: {
+          NOT_A_VALID_TOPIC: {
+            deliveryMethod: DeliveryMethod.Http,
+            callbackUrl: '/webhooks',
+          },
+        },
+      }),
+    );
+    const session = new Session({
+      id: `offline_${TEST_SHOP}`,
+      shop: TEST_SHOP,
+      isOnline: false,
+      state: 'test',
+      accessToken: 'totally_real_token',
+    });
+
+    await mockExternalRequests(
+      {
+        request: new Request(GRAPHQL_URL, {
+          method: 'POST',
+          body: 'webhookSubscriptions',
+        }),
+        response: new Response(
+          JSON.stringify(mockResponses.EMPTY_WEBHOOK_RESPONSE),
+        ),
+      },
+      {
+        request: new Request(GRAPHQL_URL, {
+          method: 'POST',
+          body: 'webhookSubscriptionCreate',
+        }),
+        response: new Response(
+          JSON.stringify({errors: [{extensions: {code: 'FAILED_REQUEST'}}]}),
+        ),
+      },
+    );
+
+    // THEN
+    expect(shopify.registerWebhooks({session})).rejects.toThrowError(
+      GraphqlQueryError,
+    );
+  });
 });
diff --git a/packages/shopify-app-remix/src/server/authenticate/webhooks/register.ts b/packages/shopify-app-remix/src/server/authenticate/webhooks/register.ts
@@ -4,26 +4,45 @@ import type {RegisterWebhooksOptions} from './types';
 
 export function registerWebhooksFactory({api, logger}: BasicParams) {
   return async function registerWebhooks({session}: RegisterWebhooksOptions) {
-    return api.webhooks.register({session}).then((response) => {
-      Object.entries(response).forEach(([topic, topicResults]) => {
-        topicResults.forEach(({success, ...rest}) => {
-          if (success) {
-            logger.debug('Registered webhook', {
-              topic,
-              shop: session.shop,
-              operation: rest.operation,
-            });
-          } else {
-            logger.error('Failed to register webhook', {
-              topic,
-              shop: session.shop,
-              result: JSON.stringify(rest.result),
-            });
-          }
+    return api.webhooks
+      .register({session})
+      .then((response) => {
+        Object.entries(response).forEach(([topic, topicResults]) => {
+          topicResults.forEach(({success, ...rest}) => {
+            if (success) {
+              logger.debug('Registered webhook', {
+                topic,
+                shop: session.shop,
+                operation: rest.operation,
+              });
+            } else {
+              logger.error('Failed to register webhook', {
+                topic,
+                shop: session.shop,
+                result: JSON.stringify(rest.result),
+              });
+            }
+          });
         });
-      });
 
-      return response;
-    });
+        return response;
+      })
+      .catch((error) => {
+        const graphQLErrors: {extensions: {code?: string}}[] =
+          error.body?.errors?.graphQLErrors || [];
+
+        const throttled = graphQLErrors.find(
+          ({extensions: {code}}) => code === 'THROTTLED',
+        );
+
+        if (throttled) {
+          logger.error('Failed to register webhooks', {
+            shop: session.shop,
+            error: JSON.stringify(error),
+          });
+        } else {
+          throw error;
+        }
+      });
   };
 }
diff --git a/packages/shopify-app-remix/src/server/types.ts b/packages/shopify-app-remix/src/server/types.ts
@@ -66,7 +66,7 @@ interface JSONArray extends Array<JSONValue> {}
 
 type RegisterWebhooks = (
   options: RegisterWebhooksOptions,
-) => Promise<RegisterReturn>;
+) => Promise<RegisterReturn | void>;
 
 export enum LoginErrorType {
   MissingShop = 'MISSING_SHOP',

diff --git a/packages/shopify-app-session-storage-dynamodb/package.json b/packages/shopify-app-session-storage-dynamodb/package.json
@@ -37,13 +37,13 @@
     "tslib": "^2.6.2"
   },
   "peerDependencies": {
-    "@shopify/shopify-api": "^9.0.1",
+    "@shopify/shopify-api": "^9.0.2",
     "@shopify/shopify-app-session-storage": "^2.0.3"
   },
   "devDependencies": {
     "@shopify/eslint-plugin": "^42.1.0",
     "@shopify/prettier-config": "^1.1.2",
-    "@shopify/shopify-api": "^9.0.1",
+    "@shopify/shopify-api": "^9.0.2",
     "@shopify/shopify-app-session-storage": "^2.0.3",
     "@shopify/shopify-app-session-storage-test-utils": "^1.0.3",
     "eslint": "^8.55.0",

diff --git a/packages/shopify-app-session-storage-kv/package.json b/packages/shopify-app-session-storage-kv/package.json
@@ -36,7 +36,7 @@
     "tslib": "^2.6.2"
   },
   "peerDependencies": {
-    "@shopify/shopify-api": "^9.0.1",
+    "@shopify/shopify-api": "^9.0.2",
     "@shopify/shopify-app-session-storage": "^2.0.3"
   },
   "devDependencies": {

diff --git a/packages/shopify-app-session-storage-memory/package.json b/packages/shopify-app-session-storage-memory/package.json
@@ -33,7 +33,7 @@
     "tslib": "^2.6.2"
   },
   "peerDependencies": {
-    "@shopify/shopify-api": "^9.0.1",
+    "@shopify/shopify-api": "^9.0.2",
     "@shopify/shopify-app-session-storage": "^2.0.3"
   },
   "devDependencies": {

diff --git a/packages/shopify-app-session-storage-mongodb/package.json b/packages/shopify-app-session-storage-mongodb/package.json
@@ -35,7 +35,7 @@
     "tslib": "^2.6.2"
   },
   "peerDependencies": {
-    "@shopify/shopify-api": "^9.0.1",
+    "@shopify/shopify-api": "^9.0.2",
     "@shopify/shopify-app-session-storage": "^2.0.3"
   },
   "devDependencies": {

diff --git a/packages/shopify-app-session-storage-mysql/package.json b/packages/shopify-app-session-storage-mysql/package.json
@@ -36,7 +36,7 @@
     "tslib": "^2.6.2"
   },
   "peerDependencies": {
-    "@shopify/shopify-api": "^9.0.1",
+    "@shopify/shopify-api": "^9.0.2",
     "@shopify/shopify-app-session-storage": "^2.0.3"
   },
   "devDependencies": {

diff --git a/packages/shopify-app-session-storage-postgresql/package.json b/packages/shopify-app-session-storage-postgresql/package.json
@@ -37,7 +37,7 @@
     "tslib": "^2.6.2"
   },
   "peerDependencies": {
-    "@shopify/shopify-api": "^9.0.1",
+    "@shopify/shopify-api": "^9.0.2",
     "@shopify/shopify-app-session-storage": "^2.0.3"
   },
   "devDependencies": {

diff --git a/packages/shopify-app-session-storage-prisma/package.json b/packages/shopify-app-session-storage-prisma/package.json
@@ -35,13 +35,13 @@
   },
   "peerDependencies": {
     "@prisma/client": "^4.13.0",
-    "@shopify/shopify-api": "^9.0.1",
+    "@shopify/shopify-api": "^9.0.2",
     "@shopify/shopify-app-session-storage": "^2.0.3",
     "prisma": "^4.13.0"
   },
   "devDependencies": {
     "@prisma/client": "^4.13.0",
-    "@shopify/shopify-api": "^9.0.1",
+    "@shopify/shopify-api": "^9.0.2",
     "@shopify/shopify-app-session-storage": "^2.0.3",
     "prisma": "^4.13.0",
     "@shopify/eslint-plugin": "^42.1.0",

diff --git a/packages/shopify-app-session-storage-redis/package.json b/packages/shopify-app-session-storage-redis/package.json
@@ -36,7 +36,7 @@
     "tslib": "^2.6.2"
   },
   "peerDependencies": {
-    "@shopify/shopify-api": "^9.0.1",
+    "@shopify/shopify-api": "^9.0.2",
     "@shopify/shopify-app-session-storage": "^2.0.3"
   },
   "devDependencies": {

diff --git a/packages/shopify-app-session-storage-sqlite/package.json b/packages/shopify-app-session-storage-sqlite/package.json
@@ -36,7 +36,7 @@
     "tslib": "^2.6.2"
   },
   "peerDependencies": {
-    "@shopify/shopify-api": "^9.0.1",
+    "@shopify/shopify-api": "^9.0.2",
     "@shopify/shopify-app-session-storage": "^2.0.3"
   },
   "devDependencies": {

diff --git a/packages/shopify-app-session-storage-test-utils/package.json b/packages/shopify-app-session-storage-test-utils/package.json
@@ -38,7 +38,7 @@
     "tslib": "^2.6.2"
   },
   "peerDependencies": {
-    "@shopify/shopify-api": "^9.0.1",
+    "@shopify/shopify-api": "^9.0.2",
     "@shopify/shopify-app-session-storage": "^2.0.3"
   },
   "devDependencies": {

diff --git a/packages/shopify-app-session-storage/package.json b/packages/shopify-app-session-storage/package.json
@@ -34,7 +34,7 @@
     "tslib": "^2.6.2"
   },
   "peerDependencies": {
-    "@shopify/shopify-api": "^9.0.1"
+    "@shopify/shopify-api": "^9.0.2"
   },
   "devDependencies": {
     "@shopify/eslint-plugin": "^42.1.0",