Merge pull request #18806 from mozilla/improve-gql-breadcrumbs

task(settings): Improve error handling for gql errors and network errors

Author: dschom

packages/fxa-settings/src/lib/gql.test.ts

@@ -7,7 +7,7 @@ import { Operation, NextLink, ServerError } from '@apollo/client/core';
 import { GraphQLError } from 'graphql';
 import { cache } from './cache';
 import { GET_LOCAL_SIGNED_IN_STATUS } from '../components/App/gql';
-import sentryMetrics from 'fxa-shared/sentry/browser';
+import * as Sentry from '@sentry/browser';
 
 describe('errorHandler', () => {
   beforeAll(() => {
@@ -26,8 +26,10 @@ describe('errorHandler', () => {
           originalError: { error: 'Unauthorized' },
         }),
       ],
-      operation: null as any as Operation,
-      forward: null as any as NextLink,
+      operation: {
+        operationName: 'foo',
+      } as Operation,
+      forward: jest.fn(),
     };
 
     errorHandler(errorResponse);
@@ -38,21 +40,55 @@ describe('errorHandler', () => {
     });
   });
 
+  it('adds breadcrumb on graphql error', () => {
+    const gqlError = new GraphQLError('Foo', null, null, null, null, null, {
+      originalError: { error: 'Boom' },
+    });
+    const errorResponse: ErrorResponse = {
+      graphQLErrors: [gqlError],
+      operation: {
+        operationName: 'Foo',
+      } as Operation,
+      forward: jest.fn(),
+    };
+
+    const addBreadcrumbMock = jest.fn();
+    jest.spyOn(Sentry, 'addBreadcrumb').mockImplementation(addBreadcrumbMock);
+
+    errorHandler(errorResponse);
+
+    expect(addBreadcrumbMock).toHaveBeenCalledWith({
+      category: 'graphql.error',
+      message: 'Foo',
+      level: 'error',
+      data: {
+        operation: 'Foo',
+        graphQLError: gqlError,
+      },
+    });
+  });
+
   it('logs network errors to Sentry', () => {
     let networkError: any;
     networkError = new Error('Network error') as ServerError;
     const errorResponse: ErrorResponse = {
       networkError,
-      operation: null as any as Operation,
-      forward: null as any as NextLink,
+      operation: {
+        operationName: 'foo',
+      } as any as Operation,
+      forward: jest.fn() as NextLink,
     };
     const captureExceptionMock = jest.fn();
     jest
-      .spyOn(sentryMetrics, 'captureException')
+      .spyOn(Sentry, 'captureException')
       .mockImplementation(captureExceptionMock);
 
     errorHandler(errorResponse);
 
-    expect(captureExceptionMock).toHaveBeenCalledWith(networkError);
+    expect(captureExceptionMock).toHaveBeenCalledWith(networkError, {
+      extra: {
+        operation: 'foo',
+      },
+    });
   });
 });

packages/fxa-settings/src/lib/gql.ts

@@ -15,7 +15,7 @@ import { BatchHttpLink } from '@apollo/client/link/batch-http';
 import { cache, sessionToken, typeDefs } from './cache';
 import { GraphQLError, GraphQLFormattedError } from 'graphql';
 import { GET_LOCAL_SIGNED_IN_STATUS } from '../components/App/gql';
-import sentryMetrics from 'fxa-shared/sentry/browser';
+import * as Sentry from '@sentry/browser';
 
 /**
  * These operation names either require auth with a valid session token
@@ -77,7 +77,11 @@ const afterwareLink = new ApolloLink((operation, forward) => {
   });
 });
 
-export const errorHandler: ErrorHandler = ({ graphQLErrors, networkError }) => {
+export const errorHandler: ErrorHandler = ({
+  graphQLErrors,
+  networkError,
+  operation,
+}) => {
   if (graphQLErrors) {
     for (const error of graphQLErrors) {
       if (isUnauthorizedError(error)) {
@@ -99,16 +103,51 @@ export const errorHandler: ErrorHandler = ({ graphQLErrors, networkError }) => {
             data: { isSignedIn: false },
           });
         }
+      } else {
+        // Add error as bread crumb, so if there's a down stream exception, we can
+        // see potential gql problems.
+        Sentry.addBreadcrumb({
+          category: 'graphql.error',
+          message: error.message,
+          level: 'error',
+          data: {
+            operation: operation.operationName,
+            graphQLError: error,
+          },
+        });
       }
     }
-    console.error('graphQLErrors', graphQLErrors);
   }
+
   if (networkError) {
-    sentryMetrics.captureException(networkError);
-    console.error('networkError', networkError);
+    Sentry.captureException(networkError, {
+      extra: {
+        operation: operation.operationName,
+      },
+    });
   }
 };
 
+const sentryLink = new ApolloLink((operation, forward) => {
+  const { operationName, variables } = operation;
+
+  return forward(operation).map((response) => {
+    // Log the response
+    Sentry.addBreadcrumb({
+      category: 'graphql.response',
+      message: operationName || 'Unnamed operation',
+      level: 'info',
+      data: {
+        request: {
+          variables,
+        },
+      },
+    });
+
+    return response;
+  });
+});
+
 let apolloClientInstance: ApolloClient<NormalizedCacheObject>;
 export function createApolloClient(gqlServerUri: string) {
   if (apolloClientInstance) {
@@ -158,6 +197,7 @@ export function createApolloClient(gqlServerUri: string) {
     cache,
     link: from([
       errorLink,
+      sentryLink,
       sessionTokenCheckLink,
       authLink,
       afterwareLink,

packages/fxa-settings/src/lib/nimbus/index.ts

@@ -54,6 +54,10 @@ export async function initializeNimbus(
       return (await resp.json()) as NimbusResult;
     }
   } catch (err) {
+    // Important, if this fails it will just show up in Sentry as a
+    // TypeError: NetworkError when attempting to fetch resource.
+    // Look at the previous fetch bread crumb to understand what
+    // request is actually failing.
     Sentry.captureException(err, {
       tags: {
         source: 'nimbus-experiments',