fix(shopify-api): 301 redirect follows with initial method

Shopify · Jan 22, 2025 · cb33f46 · cb33f46
1 parent f905b0f
commit cb33f46
Show file tree

Hide file tree

Showing 3 changed files with 63 additions and 2 deletions.
diff --git a/packages/apps/shopify-api/adapters/__e2etests__/test_apps/test-dummy-shopify-server.ts b/packages/apps/shopify-api/adapters/__e2etests__/test_apps/test-dummy-shopify-server.ts
@@ -72,6 +72,22 @@ const tests: Record<string | number, Test> = {
       }),
     }),
   },
+  301: {
+    expectedRequest: initTestRequest({method: 'post'}),
+    testResponse: initTestResponse({
+      statusCode: 301,
+      headers: {
+        location: '/url/path/301resolved',
+      },
+    }),
+  },
+  '301resolved': {
+    expectedRequest: initTestRequest({method: 'post'}),
+    testResponse: initTestResponse({
+      body: 'followed redirect',
+      statusCode: 204,
+    }),
+  },
   400: {
     expectedRequest: initTestRequest(),
     testResponse: initTestResponse({
@@ -98,6 +114,13 @@ const tests: Record<string | number, Test> = {
       body: JSON.stringify({}),
     }),
   },
+  405: {
+    expectedRequest: initTestRequest(),
+    testResponse: initTestResponse({
+      statusCode: 405,
+      statusText: 'Wrong method',
+    }),
+  },
   417: {
     expectedRequest: initTestRequest(),
     testResponse: initTestResponse({
@@ -162,6 +185,12 @@ const server = createServer((req: IncomingMessage, res: ServerResponse) => {
   const expectedRequest = test.expectedRequest;
   let testResponse = test.testResponse;
 
+  if (code.startsWith('301')) {
+    if (expectedRequest?.method !== req.method?.toLowerCase()) {
+      testResponse = tests['405'].testResponse;
+    }
+  }
+
   if (matchHeaders(receivedHeaders as Headers, expectedRequest.headers)) {
     if (code === 'retries' && retryCount < 2) {
       testResponse = tests['429'].testResponse;

diff --git a/packages/apps/shopify-api/adapters/__e2etests__/test_suite.ts b/packages/apps/shopify-api/adapters/__e2etests__/test_suite.ts
@@ -145,6 +145,18 @@ export const testSuite = [
       expectedResponse: initTestResponse(),
     },
   },
+  {
+    name: 'gracefully handles 301 redirect',
+    config: {
+      testRequest: initTestRequest({
+        url: '/url/path/301',
+        type: TestType.Graphql,
+      }),
+      expectedResponse: initTestResponse({
+        statusCode: 204,
+      }),
+    },
+  },
   {
     name: 'gracefully handles 403 error',
     config: {

diff --git a/packages/apps/shopify-api/adapters/node/index.ts b/packages/apps/shopify-api/adapters/node/index.ts
@@ -21,8 +21,28 @@ import {
   nodeRuntimeString,
 } from './adapter';
 
-// For the purposes of this package, fetch correctly implements everything we need
-setAbstractFetchFunc(fetch as any as AbstractFetchFunc);
+// node-fetch redirects post requests as get requests on 301, 302 or 303 status codes
+// this does not work for graphql requests, so we need to manually handle redirects
+const fetchWithRedirect: AbstractFetchFunc = async (url, init) => {
+  const fetchOptions = {
+    ...init,
+    redirect: 'manual',
+  } satisfies RequestInit;
+
+  const response = await (fetch as any as AbstractFetchFunc)(url, fetchOptions);
+
+  if (
+    (response.status === 301 || response.status === 302) &&
+    response.headers.has('location')
+  ) {
+    const location = response.headers.get('location')!;
+    return fetchWithRedirect(location, init);
+  }
+
+  return response;
+};
+
+setAbstractFetchFunc(fetchWithRedirect);
 setAbstractConvertRequestFunc(nodeConvertRequest);
 setAbstractConvertIncomingResponseFunc(nodeConvertIncomingResponse);
 setAbstractConvertResponseFunc(nodeConvertAndSendResponse);