Error handling across Hydrogen (#1645)

Co-authored-by: Fran Dios <fran.dios@shopify.com>
Shopify · Jan 19, 2024 · 400bfee · 400bfee
1 parent 335375a
commit 400bfee
Show file tree

Hide file tree

Showing 46 changed files with 326 additions and 188 deletions.
diff --git a/.changeset/happy-pigs-collect.md b/.changeset/happy-pigs-collect.md
@@ -0,0 +1,22 @@
+---
+'@shopify/hydrogen': patch
+'@shopify/cli-hydrogen': patch
+---
+
+Better Hydrogen error handling
+
+* Fix storefront client throwing on partial successful errors
+* Fix subrequest profiler to better display network errors with url information for SFAPI requests
+
+### Breaking change
+
+ Mutation methods of `createCartHandler` used to return an `errors` object that contains `userErrors`. This is now changed back to `userErrors` to be consistent with SFAPI schema.
+
+ The `errors` object will now be used for Graphql execution errors.
+
+ `storefront.isApiError` is deprecated.
+
+ Updated types:
+
+ * `cart.get()` used to return a `Cart` type. Now it returns `CartReturn` type to accommodate the `errors` object
+ * All other `cart` methods (ie. `cart.addLines`) used to return a `CartQueryData` type. Now it returns `CartQueryDataReturn` type to accommodate the `errors` object.
diff --git a/examples/multipass/app/routes/cart.tsx b/examples/multipass/app/routes/cart.tsx
@@ -1,6 +1,6 @@
 import {Await, type MetaFunction} from '@remix-run/react';
 import {Suspense} from 'react';
-import type {CartQueryData} from '@shopify/hydrogen';
+import type {CartQueryDataReturn} from '@shopify/hydrogen';
 import {CartForm} from '@shopify/hydrogen';
 import {json, type ActionFunctionArgs} from '@shopify/remix-oxygen';
 import {CartMain} from '~/components/Cart';
@@ -25,7 +25,7 @@ export async function action({request, context}: ActionFunctionArgs) {
   }
 
   let status = 200;
-  let result: CartQueryData;
+  let result: CartQueryDataReturn;
 
   switch (action) {
     case CartForm.ACTIONS.LinesAdd:

diff --git a/examples/multipass/app/routes/search.tsx b/examples/multipass/app/routes/search.tsx
@@ -21,7 +21,7 @@ export async function loader({request, context}: LoaderFunctionArgs) {
     };
   }
 
-  const data = await context.storefront.query(SEARCH_QUERY, {
+  const {errors, ...data} = await context.storefront.query(SEARCH_QUERY, {
     variables: {
       query: searchTerm,
       ...variables,

diff --git a/examples/subscriptions/app/routes/cart.tsx b/examples/subscriptions/app/routes/cart.tsx
@@ -1,6 +1,6 @@
 import {Await, type MetaFunction} from '@remix-run/react';
 import {Suspense} from 'react';
-import type {CartQueryData} from '@shopify/hydrogen';
+import type {CartQueryDataReturn} from '@shopify/hydrogen';
 import {CartForm} from '@shopify/hydrogen';
 import {json, type ActionFunctionArgs} from '@shopify/remix-oxygen';
 import {CartMain} from '~/components/Cart';
@@ -22,7 +22,7 @@ export async function action({request, context}: ActionFunctionArgs) {
   }
 
   let status = 200;
-  let result: CartQueryData;
+  let result: CartQueryDataReturn;
 
   switch (action) {
     case CartForm.ACTIONS.LinesAdd:

diff --git a/packages/cli/src/lib/request-events.ts b/packages/cli/src/lib/request-events.ts
@@ -60,6 +60,7 @@ export type H2OEvent = {
     key?: string | readonly unknown[];
   };
   displayName?: string;
+  url?: string;
 };
 
 async function getRequestInfo(request: RequestKind) {
@@ -104,19 +105,27 @@ export function createLogRequestEvent(options?: {absoluteBundlePath?: string}) {
       return createResponse<R>();
     }
 
-    const {eventType, purpose, graphql, stackInfo, ...data} =
-      await getRequestInfo(request);
+    const {
+      url: displayUrl,
+      displayName: displayNameData,
+      eventType,
+      purpose,
+      graphql,
+      stackInfo,
+      ...data
+    } = await getRequestInfo(request);
 
     let graphiqlLink = '';
-    let description = request.url;
+    let descriptionUrl = request.url;
+    let displayName = displayNameData;
 
     if (eventType === 'subrequest') {
-      description =
+      displayName =
+        displayName ||
         graphql?.query
           .match(/(query|mutation)\s+(\w+)/)?.[0]
-          ?.replace(/\s+/, ' ') ||
-        decodeURIComponent(url.search.slice(1)) ||
-        request.url;
+          ?.replace(/\s+/, ' ');
+      descriptionUrl = displayUrl || request.url;
 
       if (graphql) {
         graphiqlLink = getGraphiQLUrl({graphql});
@@ -150,7 +159,8 @@ export function createLogRequestEvent(options?: {absoluteBundlePath?: string}) {
       event: EVENT_MAP[eventType] || eventType,
       data: JSON.stringify({
         ...data,
-        url: `${purpose} ${description}`.trim(),
+        displayName,
+        url: `${purpose} ${descriptionUrl}`.trim(),
         graphiqlLink,
         stackLine,
         stackLink,

diff --git a/packages/hydrogen/src/cache/fetch.ts b/packages/hydrogen/src/cache/fetch.ts
@@ -21,6 +21,7 @@ import {
 export type CacheKey = string | readonly unknown[];
 
 export type FetchDebugInfo = {
+  url?: string;
   requestId?: string | null;
   graphql?: string | null;
   purpose?: string | null;
@@ -134,7 +135,6 @@ export async function runWithCache<T = unknown>(
         }) => {
           globalThis.__H2O_LOG_EVENT?.({
             eventType: 'subrequest',
-            url: debugData?.url || getKeyUrl(key),
             startTime: overrideStartTime || startTime,
             cacheStatus,
             responsePayload: (result && result[0]) || result,
@@ -146,6 +146,7 @@ export async function runWithCache<T = unknown>(
             },
             waitUntil,
             ...debugInfo,
+            url: debugData?.url || debugInfo?.url,
             displayName: debugInfo?.displayName || debugData?.displayName,
           } as any);
         }
@@ -271,11 +272,7 @@ export async function fetchWithServerCache(
           data = await response.text();
         } catch {
           // Getting a response without a valid body
-          throw new Error(
-            `Storefront API response code: ${
-              response.status
-            } (Request Id: ${response.headers.get('x-request-id')})`,
-          );
+          return toSerializableResponse('', response);
         }
       }
 

diff --git a/packages/hydrogen/src/cart/CartForm.custom.example.tsx b/packages/hydrogen/src/cart/CartForm.custom.example.tsx
@@ -1,6 +1,6 @@
 import {type ActionFunctionArgs, json} from '@remix-run/server-runtime';
 import {
-  type CartQueryData,
+  type CartQueryDataReturn,
   type HydrogenCart,
   CartForm,
 } from '@shopify/hydrogen';
@@ -36,7 +36,7 @@ export async function action({request, context}: ActionFunctionArgs) {
   const {action, inputs} = CartForm.getFormInput(formData);
 
   let status = 200;
-  let result: CartQueryData;
+  let result: CartQueryDataReturn;
 
   if (action === 'CustomEditInPlace') {
     result = await cart.addLines(inputs.addLines as CartLineInput[]);

diff --git a/packages/hydrogen/src/cart/CartForm.example.tsx b/packages/hydrogen/src/cart/CartForm.example.tsx
@@ -1,6 +1,6 @@
 import {type ActionFunctionArgs, json} from '@remix-run/server-runtime';
 import {
-  type CartQueryData,
+  type CartQueryDataReturn,
   type HydrogenCart,
   CartForm,
 } from '@shopify/hydrogen';
@@ -35,7 +35,7 @@ export async function action({request, context}: ActionFunctionArgs) {
   const {action, inputs} = CartForm.getFormInput(formData);
 
   let status = 200;
-  let result: CartQueryData;
+  let result: CartQueryDataReturn;
 
   if (action === CartForm.ACTIONS.LinesUpdate) {
     result = await cart.updateLines(inputs.lines);

diff --git a/packages/hydrogen/src/cart/CartForm.fetcher.example.tsx b/packages/hydrogen/src/cart/CartForm.fetcher.example.tsx
@@ -1,7 +1,7 @@
 import {useFetcher} from '@remix-run/react';
 import {type ActionFunctionArgs, json} from '@remix-run/server-runtime';
 import {
-  type CartQueryData,
+  type CartQueryDataReturn,
   type HydrogenCart,
   CartForm,
   type CartActionInput,
@@ -57,7 +57,7 @@ export async function action({request, context}: ActionFunctionArgs) {
   const {action, inputs} = CartForm.getFormInput(formData);
 
   let status = 200;
-  let result: CartQueryData;
+  let result: CartQueryDataReturn;
 
   if (action === CartForm.ACTIONS.MetafieldsSet) {
     result = await cart.setMetafields(inputs.metafields);

diff --git a/packages/hydrogen/src/cart/CartForm.input-tag.example.tsx b/packages/hydrogen/src/cart/CartForm.input-tag.example.tsx
@@ -1,6 +1,6 @@
 import {type ActionFunctionArgs, json} from '@remix-run/server-runtime';
 import {
-  type CartQueryData,
+  type CartQueryDataReturn,
   type HydrogenCart,
   CartForm,
 } from '@shopify/hydrogen';
@@ -25,7 +25,7 @@ export async function action({request, context}: ActionFunctionArgs) {
   const {action, inputs} = CartForm.getFormInput(formData);
 
   let status = 200;
-  let result: CartQueryData;
+  let result: CartQueryDataReturn;
 
   if (action === CartForm.ACTIONS.NoteUpdate) {
     result = await cart.updateNote(inputs.note);

diff --git a/packages/hydrogen/src/cart/cart-test-helper.ts b/packages/hydrogen/src/cart/cart-test-helper.ts
@@ -35,14 +35,25 @@ function storefrontMutate(
     cartId = 'c1-new-cart-id';
   }
 
-  return Promise.resolve({
-    [keyWrapper]: {
-      cart: {
-        id: cartId,
+  if (
+    keyWrapper === 'cartMetafieldsSet' ||
+    keyWrapper === 'cartMetafieldDelete'
+  ) {
+    return Promise.resolve({
+      [keyWrapper]: {
+        userErrors: [query],
       },
-      errors: [query],
-    },
-  });
+    });
+  } else {
+    return Promise.resolve({
+      [keyWrapper]: {
+        cart: {
+          id: cartId,
+        },
+        userErrors: [query],
+      },
+    });
+  }
 }
 
 export function mockHeaders(cartId?: string) {

diff --git a/packages/hydrogen/src/cart/createCartHandler.test.ts b/packages/hydrogen/src/cart/createCartHandler.test.ts
@@ -98,42 +98,42 @@ describe('createCartHandler', () => {
     });
 
     const result1 = await cart.create({});
-    expect(result1.errors?.[0]).toContain(cartMutateFragment);
+    expect(result1.userErrors?.[0]).toContain(cartMutateFragment);
 
     const result2 = await cart.addLines([]);
-    expect(result2.errors?.[0]).toContain(cartMutateFragment);
+    expect(result2.userErrors?.[0]).toContain(cartMutateFragment);
 
     const result3 = await cart.updateLines([]);
-    expect(result3.errors?.[0]).toContain(cartMutateFragment);
+    expect(result3.userErrors?.[0]).toContain(cartMutateFragment);
 
     const result4 = await cart.removeLines([]);
-    expect(result4.errors?.[0]).toContain(cartMutateFragment);
+    expect(result4.userErrors?.[0]).toContain(cartMutateFragment);
 
     const result5 = await cart.updateDiscountCodes([]);
-    expect(result5.errors?.[0]).toContain(cartMutateFragment);
+    expect(result5.userErrors?.[0]).toContain(cartMutateFragment);
 
     const result6 = await cart.updateBuyerIdentity({});
-    expect(result6.errors?.[0]).toContain(cartMutateFragment);
+    expect(result6.userErrors?.[0]).toContain(cartMutateFragment);
 
     const result7 = await cart.updateNote('');
-    expect(result7.errors?.[0]).toContain(cartMutateFragment);
+    expect(result7.userErrors?.[0]).toContain(cartMutateFragment);
 
     const result8 = await cart.updateSelectedDeliveryOption([
       {
         deliveryGroupId: 'gid://shopify/DeliveryGroup/123',
         deliveryOptionHandle: 'Postal Service',
       },
     ]);
-    expect(result8.errors?.[0]).toContain(cartMutateFragment);
+    expect(result8.userErrors?.[0]).toContain(cartMutateFragment);
 
     const result9 = await cart.updateAttributes([]);
-    expect(result9.errors?.[0]).toContain(cartMutateFragment);
+    expect(result9.userErrors?.[0]).toContain(cartMutateFragment);
 
     const result10 = await cart.setMetafields([]);
-    expect(result10.errors?.[0]).not.toContain(cartMutateFragment);
+    expect(result10.userErrors?.[0]).not.toContain(cartMutateFragment);
 
     const result11 = await cart.deleteMetafield('some.key');
-    expect(result11.errors?.[0]).not.toContain(cartMutateFragment);
+    expect(result11.userErrors?.[0]).not.toContain(cartMutateFragment);
   });
 
   it('function get has a working default implementation', async () => {

diff --git a/packages/hydrogen/src/cart/queries/cart-types.ts b/packages/hydrogen/src/cart/queries/cart-types.ts
@@ -8,7 +8,7 @@ import type {
   MetafieldsSetUserError,
   MetafieldDeleteUserError,
 } from '@shopify/hydrogen-react/storefront-api-types';
-import {type Storefront} from '../../storefront';
+import type {StorefrontApiErrors, Storefront} from '../../storefront';
 
 export type CartOptionalInput = {
   /**
@@ -45,14 +45,22 @@ export type CartQueryOptions = {
   cartFragment?: string;
 };
 
+export type CartReturn = Cart & {
+  errors?: StorefrontApiErrors;
+};
+
 export type CartQueryData = {
   cart: Cart;
-  errors?:
+  userErrors?:
     | CartUserError[]
     | MetafieldsSetUserError[]
     | MetafieldDeleteUserError[];
 };
 
+export type CartQueryDataReturn = CartQueryData & {
+  errors?: StorefrontApiErrors;
+};
+
 export type CartQueryReturn<T> = (
   requiredParams: T,
   optionalParams?: CartOptionalInput,

diff --git a/packages/hydrogen/src/cart/queries/cartAttributesUpdateDefault.test.ts b/packages/hydrogen/src/cart/queries/cartAttributesUpdateDefault.test.ts
@@ -25,6 +25,6 @@ describe('cartAttributesUpdateDefault', () => {
     const result = await cartAttribute([]);
 
     expect(result.cart).toHaveProperty('id', CART_ID);
-    expect(result.errors?.[0]).toContain(cartFragment);
+    expect(result.userErrors?.[0]).toContain(cartFragment);
   });
 });
diff --git a/packages/hydrogen/src/cart/queries/cartAttributesUpdateDefault.ts b/packages/hydrogen/src/cart/queries/cartAttributesUpdateDefault.ts
@@ -1,29 +1,32 @@
+import {StorefrontApiErrors, formatAPIResult} from '../../storefront';
 import {MINIMAL_CART_FRAGMENT, USER_ERROR_FRAGMENT} from './cart-fragments';
 import type {
   CartOptionalInput,
   CartQueryData,
+  CartQueryDataReturn,
   CartQueryOptions,
 } from './cart-types';
 import type {AttributeInput} from '@shopify/hydrogen-react/storefront-api-types';
 
 export type CartAttributesUpdateFunction = (
   attributes: AttributeInput[],
   optionalParams?: CartOptionalInput,
-) => Promise<CartQueryData>;
+) => Promise<CartQueryDataReturn>;
 
 export function cartAttributesUpdateDefault(
   options: CartQueryOptions,
 ): CartAttributesUpdateFunction {
   return async (attributes, optionalParams) => {
-    const {cartAttributesUpdate} = await options.storefront.mutate<{
+    const {cartAttributesUpdate, errors} = await options.storefront.mutate<{
       cartAttributesUpdate: CartQueryData;
+      errors: StorefrontApiErrors;
     }>(CART_ATTRIBUTES_UPDATE_MUTATION(options.cartFragment), {
       variables: {
         cartId: optionalParams?.cartId || options.getCartId(),
         attributes,
       },
     });
-    return cartAttributesUpdate;
+    return formatAPIResult(cartAttributesUpdate, errors);
   };
 }
 
@@ -38,7 +41,7 @@ export const CART_ATTRIBUTES_UPDATE_MUTATION = (
       cart {
         ...CartApiMutation
       }
-      errors: userErrors {
+      userErrors {
         ...CartApiError
       }
     }