From eee5d9274b72404dfb0ffef30d5503fd553be5fe Mon Sep 17 00:00:00 2001 From: Fran Dios Date: Wed, 7 Feb 2024 05:35:30 +0900 Subject: [PATCH] Log GraphQLErrors automatically (#1690) --------- Co-authored-by: Helen Lin --- .changeset/spotty-clouds-do.md | 5 ++ .../docs/generated/generated_docs_data.json | 51 +++++++---- packages/hydrogen/src/storefront.test.ts | 6 +- packages/hydrogen/src/storefront.ts | 46 ++++++---- packages/hydrogen/src/utils/callsites.ts | 18 +++- packages/hydrogen/src/utils/graphql.ts | 84 +++++++++++-------- 6 files changed, 137 insertions(+), 73 deletions(-) create mode 100644 .changeset/spotty-clouds-do.md diff --git a/.changeset/spotty-clouds-do.md b/.changeset/spotty-clouds-do.md new file mode 100644 index 0000000000..b1e98d4733 --- /dev/null +++ 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 index b1fe0c6748..95e32c17fd 100644 --- 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[] | undefined", + "value": "JsonGraphQLError[] | undefined", "description": "" }, "StorefrontMutations": { @@ -1912,7 +1912,7 @@ "filePath": "/storefront.ts", "syntaxKind": "TypeAliasDeclaration", "name": "StorefrontApiErrors", - "value": "ReturnType[] | undefined", + "value": "JsonGraphQLError[] | undefined", "description": "" }, "CartCreateFunction": { @@ -2848,7 +2848,7 @@ "filePath": "/storefront.ts", "syntaxKind": "TypeAliasDeclaration", "name": "StorefrontApiErrors", - "value": "ReturnType[] | undefined", + "value": "JsonGraphQLError[] | undefined", "description": "" }, "StorefrontMutations": { @@ -3364,7 +3364,7 @@ "filePath": "/storefront.ts", "syntaxKind": "TypeAliasDeclaration", "name": "StorefrontApiErrors", - "value": "ReturnType[] | undefined", + "value": "JsonGraphQLError[] | undefined", "description": "" }, "StorefrontMutations": { @@ -3880,7 +3880,7 @@ "filePath": "/storefront.ts", "syntaxKind": "TypeAliasDeclaration", "name": "StorefrontApiErrors", - "value": "ReturnType[] | undefined", + "value": "JsonGraphQLError[] | undefined", "description": "" }, "StorefrontMutations": { @@ -4396,7 +4396,7 @@ "filePath": "/storefront.ts", "syntaxKind": "TypeAliasDeclaration", "name": "StorefrontApiErrors", - "value": "ReturnType[] | undefined", + "value": "JsonGraphQLError[] | undefined", "description": "" }, "StorefrontMutations": { @@ -4905,7 +4905,7 @@ "filePath": "/storefront.ts", "syntaxKind": "TypeAliasDeclaration", "name": "StorefrontApiErrors", - "value": "ReturnType[] | undefined", + "value": "JsonGraphQLError[] | undefined", "description": "" }, "StorefrontMutations": { @@ -5372,7 +5372,7 @@ "filePath": "/storefront.ts", "syntaxKind": "TypeAliasDeclaration", "name": "StorefrontApiErrors", - "value": "ReturnType[] | undefined", + "value": "JsonGraphQLError[] | undefined", "description": "" }, "StorefrontMutations": { @@ -5888,7 +5888,7 @@ "filePath": "/storefront.ts", "syntaxKind": "TypeAliasDeclaration", "name": "StorefrontApiErrors", - "value": "ReturnType[] | undefined", + "value": "JsonGraphQLError[] | undefined", "description": "" }, "StorefrontMutations": { @@ -6397,7 +6397,7 @@ "filePath": "/storefront.ts", "syntaxKind": "TypeAliasDeclaration", "name": "StorefrontApiErrors", - "value": "ReturnType[] | undefined", + "value": "JsonGraphQLError[] | undefined", "description": "" }, "StorefrontMutations": { @@ -6913,7 +6913,7 @@ "filePath": "/storefront.ts", "syntaxKind": "TypeAliasDeclaration", "name": "StorefrontApiErrors", - "value": "ReturnType[] | undefined", + "value": "JsonGraphQLError[] | undefined", "description": "" }, "StorefrontMutations": { @@ -7422,7 +7422,7 @@ "filePath": "/storefront.ts", "syntaxKind": "TypeAliasDeclaration", "name": "StorefrontApiErrors", - "value": "ReturnType[] | undefined", + "value": "JsonGraphQLError[] | undefined", "description": "" }, "StorefrontMutations": { @@ -7938,7 +7938,7 @@ "filePath": "/storefront.ts", "syntaxKind": "TypeAliasDeclaration", "name": "StorefrontApiErrors", - "value": "ReturnType[] | undefined", + "value": "JsonGraphQLError[] | undefined", "description": "" }, "StorefrontMutations": { @@ -8447,7 +8447,7 @@ "filePath": "/storefront.ts", "syntaxKind": "TypeAliasDeclaration", "name": "StorefrontApiErrors", - "value": "ReturnType[] | 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[] | 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;\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;\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;\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;\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;\n /** Execute a GraphQL query against the Customer Account API. This method execute `handleAuthStatus()` ahead of query. */\n query?: (\n query: string,\n options: CustomerAccountQueryOptionsForDocs,\n ) => Promise;\n /** Execute a GraphQL mutation against the Customer Account API. This method execute `handleAuthStatus()` ahead of mutation. */\n mutate?: (\n mutation: string,\n options: CustomerAccountQueryOptionsForDocs,\n ) => Promise;\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;\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;\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;\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;\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;\n /** Execute a GraphQL query against the Customer Account API. This method execute `handleAuthStatus()` ahead of query. */\n query?: (\n query: string,\n options: CustomerAccountQueryOptionsForDocs,\n ) => Promise;\n /** Execute a GraphQL mutation against the Customer Account API. This method execute `handleAuthStatus()` ahead of mutation. */\n mutate?: (\n mutation: string,\n options: CustomerAccountQueryOptionsForDocs,\n ) => Promise;\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[] | undefined", + "value": "JsonGraphQLError[] | undefined", "description": "" }, "StorefrontMutations": { diff --git a/packages/hydrogen/src/storefront.test.ts b/packages/hydrogen/src/storefront.test.ts index 2e907d47bd..95e993197b 100644 --- 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 index 2191b7cfdc..39694af953 100644 --- 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[] // Equivalent to `Jsonify[]` - | undefined; +type JsonGraphQLError = ReturnType; // Equivalent to `Jsonify[]` +export type StorefrontApiErrors = JsonGraphQLError[] | undefined; type StorefrontError = { errors?: StorefrontApiErrors; @@ -172,6 +173,8 @@ type HydrogenClientProps = { 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 = @@ -216,6 +219,7 @@ export function createStorefrontClient( waitUntil, i18n, storefrontId, + logErrors = true, ...clientOptions } = options; const H2_PREFIX_WARN = '[h2:warn:createStorefrontClient] '; @@ -360,7 +364,18 @@ export function createStorefrontClient( const {data, errors} = body as GraphQLApiResponse; - return formatAPIResult(data, errors as StorefrontApiErrors); + const gqlErrors = errors?.map( + ({message, ...rest}) => + new GraphQLError(message, { + ...(rest as WritableDeep), + 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( query, stackInfo: getCallerStackLine?.(stackOffset), }), - stackOffset, + {stackOffset, logErrors}, ); }, /** @@ -419,7 +434,7 @@ export function createStorefrontClient( mutation, stackInfo: getCallerStackLine?.(stackOffset), }), - stackOffset, + {stackOffset, logErrors}, ); }, cache, @@ -480,17 +495,14 @@ const getStackOffset = } : undefined; -export function formatAPIResult(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( + data: T, + errors: StorefrontApiErrors, +): T & StorefrontError { + return { + ...data, + ...(errors && {errors}), + }; } export type CreateStorefrontClientForDocs = { diff --git a/packages/hydrogen/src/utils/callsites.ts b/packages/hydrogen/src/utils/callsites.ts index a4926305a1..9d3bb755c7 100644 --- a/packages/hydrogen/src/utils/callsites.ts +++ b/packages/hydrogen/src/utils/callsites.ts @@ -4,14 +4,17 @@ */ export function withSyncStack( promise: Promise, - stackOffset = 0, + options: { + stackOffset?: number; + logErrors?: boolean | ((error?: Error) => boolean); + } = {}, ): Promise { 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( 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) => { diff --git a/packages/hydrogen/src/utils/graphql.ts b/packages/hydrogen/src/utils/graphql.ts index 902eb51be4..f0d9277df0 100644 --- a/packages/hydrogen/src/utils/graphql.ts +++ b/packages/hydrogen/src/utils/graphql.ts @@ -1,6 +1,10 @@ import type {StorefrontApiResponseOk} from '@shopify/hydrogen-react'; import type {GenericVariables} from '@shopify/hydrogen-codegen'; +export function extractQueryName(query: string) { + return query.match(/(query|mutation)\s+([^({]*)/)?.[0]?.trim(); +} + export function minifyQuery(string: T) { return string .replace(/\s*#.*$/gm, '') // Remove GQL comments @@ -57,28 +61,49 @@ export class GraphQLError extends Error { extensions?: {[key: string]: unknown}; constructor( - message: string, + message?: string, options: Pick< GraphQLError, - 'locations' | 'path' | 'extensions' | 'stack' - > = {}, + 'locations' | 'path' | 'extensions' | 'stack' | 'cause' + > & { + query?: string; + queryVariables?: GenericVariables; + requestId?: string | null; + clientOperation?: string; + } = {}, ) { - super(message); + const h2Prefix = options.clientOperation + ? `[h2:error:${options.clientOperation}] ` + : ''; + + const enhancedMessage = + h2Prefix + + message + + (options.requestId ? ` - Request ID: ${options.requestId}` : ''); + + super(enhancedMessage); this.name = 'GraphQLError'; - Object.assign(this, options); + this.extensions = options.extensions; + this.locations = options.locations; + this.path = options.path; this.stack = options.stack || undefined; - if (process.env.NODE_ENV === 'development') { - // During dev, workerd logs show 'cause' but hides other properties. Put them in cause. - if (options.extensions || options.path) { - try { - this.cause = JSON.stringify({ - path: options.path, - locations: options.locations, - extensions: options.extensions, - }); - } catch {} - } + try { + this.cause = JSON.stringify({ + ...(typeof options.cause === 'object' ? options.cause : {}), + requestId: options.requestId, + ...(process.env.NODE_ENV === 'development' && { + path: options.path, + extensions: options.extensions, + graphql: h2Prefix && + options.query && { + query: options.query, + variables: JSON.stringify(options.queryVariables), + }, + }), + }); + } catch { + if (options.cause) this.cause = options.cause; } } @@ -152,28 +177,19 @@ export function throwErrorWithGqlLink({ ErrorConstructor = Error, client = 'storefront', }: GraphQLErrorOptions): never { - const requestId = response.headers.get('x-request-id'); const errorMessage = (typeof errors === 'string' ? errors : errors?.map?.((error) => error.message).join('\n')) || `URL: ${url}\nAPI response error: ${response.status}`; - throw new ErrorConstructor( - `[h2:error:${client}.${type}] ` + - errorMessage + - (requestId ? ` - Request ID: ${requestId}` : ''), - { - cause: JSON.stringify({ - errors, - requestId, - ...(process.env.NODE_ENV === 'development' && { - graphql: { - query, - variables: JSON.stringify(queryVariables), - }, - }), - }), - }, - ); + const gqlError = new GraphQLError(errorMessage, { + query, + queryVariables, + cause: {errors}, + clientOperation: `${client}.${type}`, + requestId: response.headers.get('x-request-id'), + }); + + throw new ErrorConstructor(gqlError.message, {cause: gqlError.cause}); }