Remove GraphQL/network error prefix from ApolloError messages

`ApolloError` already provides a way to differentiate between GraphQL and network errors (via its public `graphQLErrors` and `networkError` properties), so let's get the extra details out of the error message string itself.
apollographql · Apr 25, 2020 · 0e29cf4 · 0e29cf4
1 parent 4b512fd
commit 0e29cf4
Show file tree

Hide file tree

Showing 9 changed files with 31 additions and 34 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -47,6 +47,9 @@
 - **[BREAKING]** Apollo Client will no longer deliver "stale" results to `ObservableQuery` consumers, but will instead log more helpful errors about which cache fields were missing. <br/>
   [@benjamn](https://github.com/benjamn) in [#6058](https://github.com/apollographql/apollo-client/pull/6058)
 
+- **[BREAKING]** `ApolloError`'s thrown by Apollo Client no longer prefix error messages with `GraphQL error:` or `Network error:`. To differentiate between GraphQL/network errors, refer to `ApolloError`'s public `graphQLErrors` and `networkError` properties. <br/>
+  [@lorensr](https://github.com/lorensr) in [#3892](https://github.com/apollographql/apollo-client/pull/3892)
+
 - `InMemoryCache` now supports tracing garbage collection and eviction. Note that the signature of the `evict` method has been simplified in a potentially backwards-incompatible way. <br/>
   [@benjamn](https://github.com/benjamn) in [#5310](https://github.com/apollographql/apollo-client/pull/5310)
 

diff --git a/src/__tests__/__snapshots__/graphqlSubscriptions.ts.snap b/src/__tests__/__snapshots__/graphqlSubscriptions.ts.snap
@@ -1,5 +1,5 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`GraphQL Subscriptions should throw an error if the result has errors on it 1`] = `[Error: GraphQL error: This is an error]`;
+exports[`GraphQL Subscriptions should throw an error if the result has errors on it 1`] = `[Error: This is an error]`;
 
-exports[`GraphQL Subscriptions should throw an error if the result has errors on it 2`] = `[Error: GraphQL error: This is an error]`;
+exports[`GraphQL Subscriptions should throw an error if the result has errors on it 2`] = `[Error: This is an error]`;
diff --git a/src/__tests__/client.ts b/src/__tests__/client.ts
@@ -2388,7 +2388,7 @@ describe('client', () => {
 
     handle.subscribe({
       error(error) {
-        expect(error.message).toBe('Network error: Uh oh!');
+        expect(error.message).toBe('Uh oh!');
         resolve();
       },
     });
@@ -2483,7 +2483,7 @@ describe('client', () => {
       },
       error(error) {
         expect(count++).toBe(2);
-        expect(error.message).toBe('Network error: This is an error!');
+        expect(error.message).toBe('This is an error!');
 
         subscription.unsubscribe();
 
@@ -2536,7 +2536,7 @@ describe('client', () => {
 
     return client.query({ query }).catch(err => {
       expect(err.message).toBe(
-        'GraphQL error: Cannot query field "foo" on type "Post".',
+        'Cannot query field "foo" on type "Post".',
       );
     }).then(resolve, reject);
   });

diff --git a/src/__tests__/local-state/__snapshots__/general.ts.snap b/src/__tests__/local-state/__snapshots__/general.ts.snap
@@ -1,5 +1,5 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`Combining client and server state/operations should correctly propagate an error from a client resolver 1`] = `"Network error: Illegal Query Operation Occurred"`;
+exports[`Combining client and server state/operations should correctly propagate an error from a client resolver 1`] = `"Illegal Query Operation Occurred"`;
 
-exports[`Combining client and server state/operations should correctly propagate an error from a client resolver 2`] = `"Network error: Illegal Mutation Operation Occurred"`;
+exports[`Combining client and server state/operations should correctly propagate an error from a client resolver 2`] = `"Illegal Mutation Operation Occurred"`;
diff --git a/src/__tests__/optimistic.ts b/src/__tests__/optimistic.ts
@@ -210,7 +210,7 @@ describe('optimistic mutation results', () => {
           await promise;
         } catch (err) {
           expect(err).toBeInstanceOf(Error);
-          expect(err.message).toBe('Network error: forbidden (test error)');
+          expect(err.message).toBe('forbidden (test error)');
 
           const dataInStore = (client.cache as InMemoryCache).extract(true);
           expect((dataInStore['TodoList5'] as any).todos.length).toBe(3);
@@ -254,7 +254,7 @@ describe('optimistic mutation results', () => {
           .catch(err => {
             // it is ok to fail here
             expect(err).toBeInstanceOf(Error);
-            expect(err.message).toBe('Network error: forbidden (test error)');
+            expect(err.message).toBe('forbidden (test error)');
             return null;
           });
 
@@ -447,7 +447,7 @@ describe('optimistic mutation results', () => {
           await promise;
         } catch (err) {
           expect(err).toBeInstanceOf(Error);
-          expect(err.message).toBe('Network error: forbidden (test error)');
+          expect(err.message).toBe('forbidden (test error)');
 
           const dataInStore = (client.cache as InMemoryCache).extract(true);
           expect((dataInStore['TodoList5'] as any).todos.length).toBe(3);
@@ -491,7 +491,7 @@ describe('optimistic mutation results', () => {
           .catch(err => {
             // it is ok to fail here
             expect(err).toBeInstanceOf(Error);
-            expect(err.message).toBe('Network error: forbidden (test error)');
+            expect(err.message).toBe('forbidden (test error)');
             return null;
           });
 
@@ -1193,7 +1193,7 @@ describe('optimistic mutation results', () => {
         .catch(err => {
           // it is ok to fail here
           expect(err).toBeInstanceOf(Error);
-          expect(err.message).toEqual('Network error: forbidden (test error)');
+          expect(err.message).toEqual('forbidden (test error)');
           return null;
         });
 
@@ -1654,7 +1654,7 @@ describe('optimistic mutation results', () => {
         .catch(err => {
           // it is ok to fail here
           expect(err).toBeInstanceOf(Error);
-          expect(err.message).toBe('Network error: forbidden (test error)');
+          expect(err.message).toBe('forbidden (test error)');
           return null;
         });
 

diff --git a/src/core/__tests__/QueryManager/index.ts b/src/core/__tests__/QueryManager/index.ts
@@ -360,7 +360,7 @@ describe('QueryManager', () => {
         },
         error(error) {
           expect((error as any).graphQLErrors).toEqual([null]);
-          expect(error.message).toBe('GraphQL error: Error message not found.');
+          expect(error.message).toBe('Error message not found.');
           resolve();
         },
       },

diff --git a/src/errors/ApolloError.ts b/src/errors/ApolloError.ts
@@ -18,12 +18,12 @@ const generateErrorMessage = (err: ApolloError) => {
       const errorMessage = graphQLError
         ? graphQLError.message
         : 'Error message not found.';
-      message += `GraphQL error: ${errorMessage}\n`;
+      message += `${errorMessage}\n`;
     });
   }
 
   if (err.networkError) {
-    message += 'Network error: ' + err.networkError.message + '\n';
+    message += `${err.networkError.message}\n`;
   }
 
   // strip newline from the end of the message

diff --git a/src/errors/__tests__/ApolloError.ts b/src/errors/__tests__/ApolloError.ts
@@ -23,7 +23,6 @@ describe('ApolloError', () => {
     const apolloError = new ApolloError({
       networkError,
     });
-    expect(apolloError.message).toMatch('Network error: ');
     expect(apolloError.message).toMatch('this is an error message');
     expect(apolloError.message.split('\n').length).toBe(1);
   });
@@ -33,7 +32,6 @@ describe('ApolloError', () => {
     const apolloError = new ApolloError({
       graphQLErrors,
     });
-    expect(apolloError.message).toMatch('GraphQL error: ');
     expect(apolloError.message).toMatch('this is an error message');
     expect(apolloError.message.split('\n').length).toBe(1);
   });
@@ -45,9 +43,7 @@ describe('ApolloError', () => {
     });
     const messages = apolloError.message.split('\n');
     expect(messages.length).toBe(2);
-    expect(messages[0]).toMatch('GraphQL error');
     expect(messages[0]).toMatch('this is new');
-    expect(messages[1]).toMatch('GraphQL error');
     expect(messages[1]).toMatch('this is old');
   });
 
@@ -60,9 +56,7 @@ describe('ApolloError', () => {
     });
     const messages = apolloError.message.split('\n');
     expect(messages.length).toBe(2);
-    expect(messages[0]).toMatch('GraphQL error');
     expect(messages[0]).toMatch('graphql error message');
-    expect(messages[1]).toMatch('Network error');
     expect(messages[1]).toMatch('network error message');
   });
 

diff --git a/src/react/hooks/__tests__/useQuery.test.tsx b/src/react/hooks/__tests__/useQuery.test.tsx
@@ -532,7 +532,7 @@ describe('useQuery Hook', () => {
         const { loading, error } = useQuery(query);
         if (!loading) {
           expect(error).toBeDefined();
-          expect(error!.message).toEqual('GraphQL error: forced error');
+          expect(error!.message).toEqual('forced error');
         }
         return null;
       };
@@ -594,7 +594,7 @@ describe('useQuery Hook', () => {
           case 2:
             expect(loading).toBeFalsy();
             expect(error).toBeDefined();
-            expect(error!.message).toEqual('Network error: Oh no!');
+            expect(error!.message).toEqual('Oh no!');
             onErrorPromise.then(() => refetch());
             break;
           case 3:
@@ -648,14 +648,14 @@ describe('useQuery Hook', () => {
             break;
           case 1:
             expect(error).toBeDefined();
-            expect(error!.message).toEqual('GraphQL error: forced error');
+            expect(error!.message).toEqual('forced error');
             setTimeout(() => {
               forceUpdate(0);
             });
             break;
           case 2:
             expect(error).toBeDefined();
-            expect(error!.message).toEqual('GraphQL error: forced error');
+            expect(error!.message).toEqual('forced error');
             break;
           default: // Do nothing
         }
@@ -711,14 +711,14 @@ describe('useQuery Hook', () => {
               break;
             case 1:
               expect(error).toBeDefined();
-              expect(error!.message).toEqual('GraphQL error: forced error');
+              expect(error!.message).toEqual('forced error');
               setTimeout(() => {
                 forceUpdate(0);
               });
               break;
             case 2:
               expect(error).toBeDefined();
-              expect(error!.message).toEqual('GraphQL error: forced error');
+              expect(error!.message).toEqual('forced error');
               break;
             default: // Do nothing
           }
@@ -777,7 +777,7 @@ describe('useQuery Hook', () => {
           case 2:
             expect(loading).toBeFalsy();
             expect(error).toBeDefined();
-            expect(error!.message).toEqual('GraphQL error: an error 1');
+            expect(error!.message).toEqual('an error 1');
             setTimeout(() => {
               // catch here to avoid failing due to 'uncaught promise rejection'
               refetch().catch(() => {});
@@ -786,7 +786,7 @@ describe('useQuery Hook', () => {
           case 3:
             expect(loading).toBeFalsy();
             expect(error).toBeDefined();
-            expect(error!.message).toEqual('GraphQL error: an error 2');
+            expect(error!.message).toEqual('an error 2');
             break;
           default: // Do nothing
         }
@@ -843,9 +843,9 @@ describe('useQuery Hook', () => {
           case 2:
             expect(loading).toBeFalsy();
             expect(error).toBeDefined();
-            expect(error!.message).toEqual('GraphQL error: same error message');
+            expect(error!.message).toEqual('same error message');
             refetch().catch(error => {
-              if (error.message !== 'GraphQL error: same error message') {
+              if (error.message !== 'same error message') {
                 reject(error);
               }
             });
@@ -903,7 +903,7 @@ describe('useQuery Hook', () => {
           case 2:
             expect(loading).toBeFalsy();
             expect(error).toBeDefined();
-            expect(error!.message).toEqual('GraphQL error: same error message');
+            expect(error!.message).toEqual('same error message');
             setTimeout(() => {
               // catch here to avoid failing due to 'uncaught promise rejection'
               refetch().catch(() => {});
@@ -924,7 +924,7 @@ describe('useQuery Hook', () => {
           case 5:
             expect(loading).toBeFalsy();
             expect(error).toBeDefined();
-            expect(error!.message).toEqual('GraphQL error: same error message');
+            expect(error!.message).toEqual('same error message');
             break;
           default: // Do nothing
         }