Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log GraphQLErrors automatically #1690

Merged
merged 13 commits into from
Feb 6, 2024
5 changes: 5 additions & 0 deletions .changeset/spotty-clouds-do.md
Original file line number Diff line number Diff line change
@@ -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.
6 changes: 5 additions & 1 deletion packages/hydrogen/src/storefront.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ describe('createStorefrontClient', () => {
storefrontId,
storefrontHeaders,
publicStorefrontToken,
logErrors: false,
});

vi.mocked(fetchWithServerCache).mockResolvedValueOnce([
Expand All @@ -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'},
],
});
});
});
Expand Down
46 changes: 29 additions & 17 deletions packages/hydrogen/src/storefront.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -42,6 +43,7 @@ import {
minifyQuery,
assertQuery,
assertMutation,
extractQueryName,
throwErrorWithGqlLink,
GraphQLError,
type GraphQLApiResponse,
Expand All @@ -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;
Expand Down Expand Up @@ -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> =
Expand Down Expand Up @@ -216,6 +219,7 @@ export function createStorefrontClient<TI18n extends I18nBase>(
waitUntil,
i18n,
storefrontId,
logErrors = true,
...clientOptions
} = options;
const H2_PREFIX_WARN = '[h2:warn:createStorefrontClient] ';
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -391,7 +406,7 @@ export function createStorefrontClient<TI18n extends I18nBase>(
query,
stackInfo: getCallerStackLine?.(stackOffset),
}),
stackOffset,
{stackOffset, logErrors},
);
},
/**
Expand Down Expand Up @@ -419,7 +434,7 @@ export function createStorefrontClient<TI18n extends I18nBase>(
mutation,
stackInfo: getCallerStackLine?.(stackOffset),
}),
stackOffset,
{stackOffset, logErrors},
);
},
cache,
Expand Down Expand Up @@ -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> = {
Expand Down
18 changes: 15 additions & 3 deletions packages/hydrogen/src/utils/callsites.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, ''));
Expand All @@ -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) => {
Expand Down
84 changes: 50 additions & 34 deletions packages/hydrogen/src/utils/graphql.ts
Original file line number Diff line number Diff line change
@@ -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<T extends string>(string: T) {
return string
.replace(/\s*#.*$/gm, '') // Remove GQL comments
Expand Down Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -152,28 +177,19 @@ export function throwErrorWithGqlLink<T>({
ErrorConstructor = Error,
client = 'storefront',
}: GraphQLErrorOptions<T>): 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});
}
Loading