Log GraphQLErrors automatically (#1690)

--------- Co-authored-by: Helen Lin <helen.lin@shopify.com>
Shopify · Feb 6, 2024 · eee5d92 · eee5d92
1 parent a266436
commit eee5d92
Show file tree

Hide file tree

Showing 6 changed files with 137 additions and 73 deletions.
diff --git a/.changeset/spotty-clouds-do.md b/.changeset/spotty-clouds-do.md
@@ -0,0 +1,5 @@
+---
+'@shopify/hydrogen': patch
+---
+
+Log GraphQL errors automatically in Storefront client, with a new `logErrors: boolean` option to disable it. Add back a link to GraphiQL in the error message.
diff --git a/packages/hydrogen/docs/generated/generated_docs_data.json b/packages/hydrogen/docs/generated/generated_docs_data.json
@@ -1577,7 +1577,7 @@
             "filePath": "/storefront.ts",
             "syntaxKind": "TypeAliasDeclaration",
             "name": "StorefrontApiErrors",
-            "value": "ReturnType<GraphQLError['toJSON']>[] | undefined",
+            "value": "JsonGraphQLError[] | undefined",
             "description": ""
           },
           "StorefrontMutations": {
@@ -1912,7 +1912,7 @@
             "filePath": "/storefront.ts",
             "syntaxKind": "TypeAliasDeclaration",
             "name": "StorefrontApiErrors",
-            "value": "ReturnType<GraphQLError['toJSON']>[] | undefined",
+            "value": "JsonGraphQLError[] | undefined",
             "description": ""
           },
           "CartCreateFunction": {
@@ -2848,7 +2848,7 @@
             "filePath": "/storefront.ts",
             "syntaxKind": "TypeAliasDeclaration",
             "name": "StorefrontApiErrors",
-            "value": "ReturnType<GraphQLError['toJSON']>[] | undefined",
+            "value": "JsonGraphQLError[] | undefined",
             "description": ""
           },
           "StorefrontMutations": {
@@ -3364,7 +3364,7 @@
             "filePath": "/storefront.ts",
             "syntaxKind": "TypeAliasDeclaration",
             "name": "StorefrontApiErrors",
-            "value": "ReturnType<GraphQLError['toJSON']>[] | undefined",
+            "value": "JsonGraphQLError[] | undefined",
             "description": ""
           },
           "StorefrontMutations": {
@@ -3880,7 +3880,7 @@
             "filePath": "/storefront.ts",
             "syntaxKind": "TypeAliasDeclaration",
             "name": "StorefrontApiErrors",
-            "value": "ReturnType<GraphQLError['toJSON']>[] | undefined",
+            "value": "JsonGraphQLError[] | undefined",
             "description": ""
           },
           "StorefrontMutations": {
@@ -4396,7 +4396,7 @@
             "filePath": "/storefront.ts",
             "syntaxKind": "TypeAliasDeclaration",
             "name": "StorefrontApiErrors",
-            "value": "ReturnType<GraphQLError['toJSON']>[] | undefined",
+            "value": "JsonGraphQLError[] | undefined",
             "description": ""
           },
           "StorefrontMutations": {
@@ -4905,7 +4905,7 @@
             "filePath": "/storefront.ts",
             "syntaxKind": "TypeAliasDeclaration",
             "name": "StorefrontApiErrors",
-            "value": "ReturnType<GraphQLError['toJSON']>[] | undefined",
+            "value": "JsonGraphQLError[] | undefined",
             "description": ""
           },
           "StorefrontMutations": {
@@ -5372,7 +5372,7 @@
             "filePath": "/storefront.ts",
             "syntaxKind": "TypeAliasDeclaration",
             "name": "StorefrontApiErrors",
-            "value": "ReturnType<GraphQLError['toJSON']>[] | undefined",
+            "value": "JsonGraphQLError[] | undefined",
             "description": ""
           },
           "StorefrontMutations": {
@@ -5888,7 +5888,7 @@
             "filePath": "/storefront.ts",
             "syntaxKind": "TypeAliasDeclaration",
             "name": "StorefrontApiErrors",
-            "value": "ReturnType<GraphQLError['toJSON']>[] | undefined",
+            "value": "JsonGraphQLError[] | undefined",
             "description": ""
           },
           "StorefrontMutations": {
@@ -6397,7 +6397,7 @@
             "filePath": "/storefront.ts",
             "syntaxKind": "TypeAliasDeclaration",
             "name": "StorefrontApiErrors",
-            "value": "ReturnType<GraphQLError['toJSON']>[] | undefined",
+            "value": "JsonGraphQLError[] | undefined",
             "description": ""
           },
           "StorefrontMutations": {
@@ -6913,7 +6913,7 @@
             "filePath": "/storefront.ts",
             "syntaxKind": "TypeAliasDeclaration",
             "name": "StorefrontApiErrors",
-            "value": "ReturnType<GraphQLError['toJSON']>[] | undefined",
+            "value": "JsonGraphQLError[] | undefined",
             "description": ""
           },
           "StorefrontMutations": {
@@ -7422,7 +7422,7 @@
             "filePath": "/storefront.ts",
             "syntaxKind": "TypeAliasDeclaration",
             "name": "StorefrontApiErrors",
-            "value": "ReturnType<GraphQLError['toJSON']>[] | undefined",
+            "value": "JsonGraphQLError[] | undefined",
             "description": ""
           },
           "StorefrontMutations": {
@@ -7938,7 +7938,7 @@
             "filePath": "/storefront.ts",
             "syntaxKind": "TypeAliasDeclaration",
             "name": "StorefrontApiErrors",
-            "value": "ReturnType<GraphQLError['toJSON']>[] | undefined",
+            "value": "JsonGraphQLError[] | undefined",
             "description": ""
           },
           "StorefrontMutations": {
@@ -8447,7 +8447,7 @@
             "filePath": "/storefront.ts",
             "syntaxKind": "TypeAliasDeclaration",
             "name": "StorefrontApiErrors",
-            "value": "ReturnType<GraphQLError['toJSON']>[] | undefined",
+            "value": "JsonGraphQLError[] | undefined",
             "description": ""
           },
           "StorefrontMutations": {
@@ -8722,7 +8722,7 @@
             "filePath": "/storefront.ts",
             "syntaxKind": "TypeAliasDeclaration",
             "name": "HydrogenClientProps",
-            "value": "{\n  /** Storefront API headers. If on Oxygen, use `getStorefrontHeaders()` */\n  storefrontHeaders?: StorefrontHeaders;\n  /** An instance that implements the [Cache API](https://developer.mozilla.org/en-US/docs/Web/API/Cache) */\n  cache?: Cache;\n  /** The globally unique identifier for the Shop */\n  storefrontId?: string;\n  /** The `waitUntil` function is used to keep the current request/response lifecycle alive even after a response has been sent. It should be provided by your platform. */\n  waitUntil?: ExecutionContext['waitUntil'];\n  /** An object containing a country code and language code */\n  i18n?: TI18n;\n}",
+            "value": "{\n  /** Storefront API headers. If on Oxygen, use `getStorefrontHeaders()` */\n  storefrontHeaders?: StorefrontHeaders;\n  /** An instance that implements the [Cache API](https://developer.mozilla.org/en-US/docs/Web/API/Cache) */\n  cache?: Cache;\n  /** The globally unique identifier for the Shop */\n  storefrontId?: string;\n  /** The `waitUntil` function is used to keep the current request/response lifecycle alive even after a response has been sent. It should be provided by your platform. */\n  waitUntil?: ExecutionContext['waitUntil'];\n  /** An object containing a country code and language code */\n  i18n?: TI18n;\n  /** Whether it should print GraphQL errors automatically. Defaults to true */\n  logErrors?: boolean | ((error?: Error) => boolean);\n}",
             "description": "",
             "members": [
               {
@@ -8764,6 +8764,14 @@
                 "value": "TI18n",
                 "description": "An object containing a country code and language code",
                 "isOptional": true
+              },
+              {
+                "filePath": "/storefront.ts",
+                "syntaxKind": "PropertySignature",
+                "name": "logErrors",
+                "value": "boolean | ((error?: Error) => boolean)",
+                "description": "Whether it should print GraphQL errors automatically. Defaults to true",
+                "isOptional": true
               }
             ]
           },
@@ -9069,7 +9077,7 @@
             "filePath": "/storefront.ts",
             "syntaxKind": "TypeAliasDeclaration",
             "name": "StorefrontApiErrors",
-            "value": "ReturnType<GraphQLError['toJSON']>[] | undefined",
+            "value": "JsonGraphQLError[] | undefined",
             "description": ""
           },
           "StorefrontMutationOptionsForDocs": {
@@ -9547,7 +9555,7 @@
             "filePath": "/customer/types.ts",
             "syntaxKind": "TypeAliasDeclaration",
             "name": "CustomerAccountForDocs",
-            "value": "{\n  /** Start the OAuth login flow. This function should be called and returned from a Remix action. It redirects the customer to a Shopify login domain. It also defined the final path the customer lands on at the end of the oAuth flow with the value of the `return_to` query param. (This is automatically setup unless `customAuthStatusHandler` option is in use) */\n  login?: () => Promise<Response>;\n  /** On successful login, the customer redirects back to your app. This function validates the OAuth response and exchanges the authorization code for an access token and refresh token. It also persists the tokens on your session. This function should be called and returned from the Remix loader configured as the redirect URI within the Customer Account API settings in admin. */\n  authorize?: () => Promise<Response>;\n  /** Returns if the customer is logged in. It also checks if the access token is expired and refreshes it if needed. */\n  isLoggedIn?: () => Promise<boolean>;\n  /** Check for a not logged in customer and redirect customer to login page. The redirect can be overwritten with `customAuthStatusHandler` option. */\n  handleAuthStatus?: () => void | DataFunctionValue;\n  /** Returns CustomerAccessToken if the customer is logged in. It also run a expiry check and does a token refresh if needed. */\n  getAccessToken?: () => Promise<string | undefined>;\n  /** Logout the customer by clearing the session and redirecting to the login domain. It should be called and returned from a Remix action. The path app should redirect to after logout can be setup in Customer Account API settings in admin.*/\n  logout?: () => Promise<Response>;\n  /** Execute a GraphQL query against the Customer Account API. This method execute `handleAuthStatus()` ahead of query. */\n  query?: <TData = any>(\n    query: string,\n    options: CustomerAccountQueryOptionsForDocs,\n  ) => Promise<TData>;\n  /** Execute a GraphQL mutation against the Customer Account API. This method execute `handleAuthStatus()` ahead of mutation. */\n  mutate?: <TData = any>(\n    mutation: string,\n    options: CustomerAccountQueryOptionsForDocs,\n  ) => Promise<TData>;\n}",
+            "value": "{\n  /** Start the OAuth login flow. This function should be called and returned from a Remix action. It redirects the customer to a Shopify login domain. It also defined the final path the customer lands on at the end of the oAuth flow with the value of the `return_to` query param. (This is automatically setup unless `customAuthStatusHandler` option is in use) */\n  login?: () => Promise<Response>;\n  /** On successful login, the customer redirects back to your app. This function validates the OAuth response and exchanges the authorization code for an access token and refresh token. It also persists the tokens on your session. This function should be called and returned from the Remix loader configured as the redirect URI within the Customer Account API settings in admin. */\n  authorize?: () => Promise<Response>;\n  /** Returns if the customer is logged in. It also checks if the access token is expired and refreshes it if needed. */\n  isLoggedIn?: () => Promise<boolean>;\n  /** Check for a not logged in customer and redirect customer to login page. The redirect can be overwritten with `customAuthStatusHandler` option. */\n  handleAuthStatus?: () => void | DataFunctionValue;\n  /** Returns CustomerAccessToken if the customer is logged in. It also run a expiry check and does a token refresh if needed. */\n  getAccessToken?: () => Promise<string | undefined>;\n  /** Creates the fully-qualified URL to your store's GraphQL endpoint.*/\n  getApiUrl: () => string;\n  /** Logout the customer by clearing the session and redirecting to the login domain. It should be called and returned from a Remix action. The path app should redirect to after logout can be setup in Customer Account API settings in admin.*/\n  logout?: () => Promise<Response>;\n  /** Execute a GraphQL query against the Customer Account API. This method execute `handleAuthStatus()` ahead of query. */\n  query?: <TData = any>(\n    query: string,\n    options: CustomerAccountQueryOptionsForDocs,\n  ) => Promise<TData>;\n  /** Execute a GraphQL mutation against the Customer Account API. This method execute `handleAuthStatus()` ahead of mutation. */\n  mutate?: <TData = any>(\n    mutation: string,\n    options: CustomerAccountQueryOptionsForDocs,\n  ) => Promise<TData>;\n}",
             "description": "Below are types meant for documentation only. Ensure it stay in sync with the type above.",
             "members": [
               {
@@ -9590,6 +9598,13 @@
                 "description": "Returns CustomerAccessToken if the customer is logged in. It also run a expiry check and does a token refresh if needed.",
                 "isOptional": true
               },
+              {
+                "filePath": "/customer/types.ts",
+                "syntaxKind": "PropertySignature",
+                "name": "getApiUrl",
+                "value": "() => string",
+                "description": "Creates the fully-qualified URL to your store's GraphQL endpoint."
+              },
               {
                 "filePath": "/customer/types.ts",
                 "syntaxKind": "PropertySignature",
@@ -10639,7 +10654,7 @@
             "filePath": "/storefront.ts",
             "syntaxKind": "TypeAliasDeclaration",
             "name": "StorefrontApiErrors",
-            "value": "ReturnType<GraphQLError['toJSON']>[] | undefined",
+            "value": "JsonGraphQLError[] | undefined",
             "description": ""
           },
           "StorefrontMutations": {

diff --git a/packages/hydrogen/src/storefront.test.ts b/packages/hydrogen/src/storefront.test.ts
@@ -161,6 +161,7 @@ describe('createStorefrontClient', () => {
         storefrontId,
         storefrontHeaders,
         publicStorefrontToken,
+        logErrors: false,
       });
 
       vi.mocked(fetchWithServerCache).mockResolvedValueOnce([
@@ -174,7 +175,10 @@ describe('createStorefrontClient', () => {
       const data = await storefront.query('query {}');
       expect(data).toMatchObject({
         cart: {},
-        errors: [{message: 'first'}, {message: 'second'}],
+        errors: [
+          {message: '[h2:error:storefront.query] first'},
+          {message: '[h2:error:storefront.query] second'},
+        ],
       });
     });
   });

diff --git a/packages/hydrogen/src/storefront.ts b/packages/hydrogen/src/storefront.ts
@@ -8,6 +8,7 @@ import {
   SHOPIFY_STOREFRONT_S_HEADER,
   type StorefrontClientProps,
 } from '@shopify/hydrogen-react';
+import type {WritableDeep} from 'type-fest';
 import {fetchWithServerCache, checkGraphQLErrors} from './cache/fetch';
 import {
   SDK_VARIANT_HEADER,
@@ -42,6 +43,7 @@ import {
   minifyQuery,
   assertQuery,
   assertMutation,
+  extractQueryName,
   throwErrorWithGqlLink,
   GraphQLError,
   type GraphQLApiResponse,
@@ -63,9 +65,8 @@ export type I18nBase = {
 // Therefore, we need make TS think this is a plain object instead of
 // a class to make it work in server and client.
 // Also, Remix' `Jsonify` type is broken and can't infer types of classes properly.
-export type StorefrontApiErrors =
-  | ReturnType<GraphQLError['toJSON']>[] // Equivalent to `Jsonify<GraphQLError>[]`
-  | undefined;
+type JsonGraphQLError = ReturnType<GraphQLError['toJSON']>; // Equivalent to `Jsonify<GraphQLError>[]`
+export type StorefrontApiErrors = JsonGraphQLError[] | undefined;
 
 type StorefrontError = {
   errors?: StorefrontApiErrors;
@@ -172,6 +173,8 @@ type HydrogenClientProps<TI18n> = {
   waitUntil?: ExecutionContext['waitUntil'];
   /** An object containing a country code and language code */
   i18n?: TI18n;
+  /** Whether it should print GraphQL errors automatically. Defaults to true */
+  logErrors?: boolean | ((error?: Error) => boolean);
 };
 
 export type CreateStorefrontClientOptions<TI18n extends I18nBase> =
@@ -216,6 +219,7 @@ export function createStorefrontClient<TI18n extends I18nBase>(
     waitUntil,
     i18n,
     storefrontId,
+    logErrors = true,
     ...clientOptions
   } = options;
   const H2_PREFIX_WARN = '[h2:warn:createStorefrontClient] ';
@@ -360,7 +364,18 @@ export function createStorefrontClient<TI18n extends I18nBase>(
 
     const {data, errors} = body as GraphQLApiResponse<T>;
 
-    return formatAPIResult(data, errors as StorefrontApiErrors);
+    const gqlErrors = errors?.map(
+      ({message, ...rest}) =>
+        new GraphQLError(message, {
+          ...(rest as WritableDeep<typeof rest>),
+          clientOperation: `storefront.${errorOptions.type}`,
+          requestId: response.headers.get('x-request-id'),
+          queryVariables,
+          query,
+        }),
+    );
+
+    return formatAPIResult(data, gqlErrors);
   }
 
   return {
@@ -391,7 +406,7 @@ export function createStorefrontClient<TI18n extends I18nBase>(
             query,
             stackInfo: getCallerStackLine?.(stackOffset),
           }),
-          stackOffset,
+          {stackOffset, logErrors},
         );
       },
       /**
@@ -419,7 +434,7 @@ export function createStorefrontClient<TI18n extends I18nBase>(
             mutation,
             stackInfo: getCallerStackLine?.(stackOffset),
           }),
-          stackOffset,
+          {stackOffset, logErrors},
         );
       },
       cache,
@@ -480,17 +495,14 @@ const getStackOffset =
       }
     : undefined;
 
-export function formatAPIResult<T>(data: T, errors: StorefrontApiErrors) {
-  let result = data;
-  if (errors) {
-    result = {
-      ...data,
-      errors: errors.map(
-        (errorOptions) => new GraphQLError(errorOptions.message, errorOptions),
-      ),
-    };
-  }
-  return result as T & StorefrontError;
+export function formatAPIResult<T>(
+  data: T,
+  errors: StorefrontApiErrors,
+): T & StorefrontError {
+  return {
+    ...data,
+    ...(errors && {errors}),
+  };
 }
 
 export type CreateStorefrontClientForDocs<TI18n extends I18nBase> = {

diff --git a/packages/hydrogen/src/utils/callsites.ts b/packages/hydrogen/src/utils/callsites.ts
@@ -4,14 +4,17 @@
  */
 export function withSyncStack<T>(
   promise: Promise<T>,
-  stackOffset = 0,
+  options: {
+    stackOffset?: number;
+    logErrors?: boolean | ((error?: Error) => boolean);
+  } = {},
 ): Promise<T> {
   const syncError = new Error();
   const getSyncStack = (message: string, name = 'Error') => {
     // Remove error message, caller function and current function from the stack.
     const syncStack = (syncError.stack ?? '')
       .split('\n')
-      .slice(3 + stackOffset)
+      .slice(3 + (options.stackOffset ?? 0))
       .join('\n')
       // Sometimes stack traces show loaders with a number suffix due to ESBuild.
       .replace(/ at loader(\d+) \(/, (all, m1) => all.replace(m1, ''));
@@ -22,10 +25,19 @@ export function withSyncStack<T>(
   return promise
     .then((result: any) => {
       if (result?.errors && Array.isArray(result.errors)) {
+        const logErrors =
+          typeof options.logErrors === 'function'
+            ? options.logErrors
+            : () => options.logErrors ?? false;
+
         result.errors.forEach((error: Error) => {
-          if (error) error.stack = getSyncStack(error.message, error.name);
+          if (error) {
+            error.stack = getSyncStack(error.message, error.name);
+            if (logErrors(error)) console.error(error);
+          }
         });
       }
+
       return result;
     })
     .catch((error: Error) => {