From 4683a0ecbe32b90e170c9c86131ff603d2b185f1 Mon Sep 17 00:00:00 2001 From: Rezaan Syed Date: Wed, 17 Jan 2024 11:40:20 -0500 Subject: [PATCH] Revert "Log throttling errors on webhook registration" This reverts commit 7888756951fa69af2a12a8e9a6063da9486181c1. --- .../webhooks/__tests__/mock-responses.ts | 11 --- .../webhooks/__tests__/register.test.ts | 99 +------------------ .../server/authenticate/webhooks/register.ts | 55 ++++------- .../shopify-app-remix/src/server/types.ts | 2 +- 4 files changed, 21 insertions(+), 146 deletions(-) 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 index fd0533599b..5377ca670a 100644 --- 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,14 +32,3 @@ 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 index c8cec6fdc4..b3dfd241ad 100644 --- 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, GraphqlQueryError, Session} from '@shopify/shopify-api'; +import {DeliveryMethod, Session} from '@shopify/shopify-api'; import {shopifyApp} from '../../..'; import { @@ -110,101 +110,4 @@ 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 index e661742c38..0acc99c6a7 100644 --- a/packages/shopify-app-remix/src/server/authenticate/webhooks/register.ts +++ b/packages/shopify-app-remix/src/server/authenticate/webhooks/register.ts @@ -4,43 +4,26 @@ 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; - }) - .catch((error) => { - if ( - error.response?.errors?.find( - (responseError: {extensions: {code: string}}) => - responseError?.extensions?.code === 'THROTTLED', - ) - ) { - logger.error('Failed to register webhooks', { - shop: session.shop, - error: JSON.stringify(error), - }); - } else { - throw error; - } }); + + return response; + }); }; } diff --git a/packages/shopify-app-remix/src/server/types.ts b/packages/shopify-app-remix/src/server/types.ts index e3cfaa55ac..9d942981f5 100644 --- 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 {} type RegisterWebhooks = ( options: RegisterWebhooksOptions, -) => Promise; +) => Promise; export enum LoginErrorType { MissingShop = 'MISSING_SHOP',