Merge pull request #5287 from apollographql/glasser/emit-error-and-th…

…row-nope Make error handling around APQs more consistent
apollographql · Jun 9, 2021 · e0c81c8 · e0c81c8
2 parents fd93b76 + 75dd2e0
commit e0c81c8
Show file tree

Hide file tree

Showing 12 changed files with 305 additions and 453 deletions.
diff --git a/packages/apollo-server-core/src/__tests__/errors.test.ts b/packages/apollo-server-core/src/__tests__/errors.test.ts
@@ -8,9 +8,6 @@ import {
   ValidationError,
   UserInputError,
   SyntaxError,
-  hasPersistedQueryError,
-  PersistedQueryNotFoundError,
-  PersistedQueryNotSupportedError,
 } from 'apollo-server-errors';
 
 describe('Errors', () => {
@@ -191,37 +188,4 @@ describe('Errors', () => {
       expect(formattedError.extensions.exception.field2).toEqual('property2');
     });
   });
-  describe('hasPersistedQueryError', () => {
-    it('should return true if errors contain error of type PersistedQueryNotFoundError', () => {
-      const errors = [
-        new PersistedQueryNotFoundError(),
-        new AuthenticationError('401'),
-      ];
-      const result = hasPersistedQueryError(errors);
-      expect(result).toBe(true);
-    });
-
-    it('should return true if errors contain error of type PersistedQueryNotSupportedError', () => {
-      const errors = [
-        new PersistedQueryNotSupportedError(),
-        new AuthenticationError('401'),
-      ];
-      const result = hasPersistedQueryError(errors);
-      expect(result).toBe(true);
-    });
-
-    it('should return false if errors does not contain PersistedQuery error', () => {
-      const errors = [
-        new ForbiddenError('401'),
-        new AuthenticationError('401'),
-      ];
-      const result = hasPersistedQueryError(errors);
-      expect(result).toBe(false);
-    });
-
-    it('should return false if illegally passed an object instead of an array', () => {
-      const result = hasPersistedQueryError({} as any);
-      expect(result).toBe(false);
-    });
-  });
 });
diff --git a/packages/apollo-server-core/src/__tests__/runHttpQuery.test.ts b/packages/apollo-server-core/src/__tests__/runHttpQuery.test.ts
@@ -2,16 +2,7 @@ import MockReq = require('mock-req');
 
 import { GraphQLSchema, GraphQLObjectType, GraphQLString } from 'graphql';
 
-import {
-  runHttpQuery,
-  HttpQueryError,
-  throwHttpGraphQLError,
-} from '../runHttpQuery';
-import {
-  PersistedQueryNotFoundError,
-  PersistedQueryNotSupportedError,
-  ForbiddenError,
-} from 'apollo-server-errors';
+import { runHttpQuery, HttpQueryError } from '../runHttpQuery';
 import { generateSchemaHash } from '../utils/schemaHash';
 
 const queryType = new GraphQLObjectType({
@@ -52,40 +43,18 @@ describe('runHttpQuery', () => {
       expect.assertions(2);
       return runHttpQuery([], noQueryRequest).catch((err: HttpQueryError) => {
         expect(err.statusCode).toEqual(400);
-        expect(err.message).toEqual('Must provide query string.');
+        expect(err.message).toEqual(
+          JSON.stringify({
+            errors: [
+              {
+                message:
+                  'GraphQL operations must contain a `query` or a `persistedQuery` extension.',
+                extensions: { code: 'INTERNAL_SERVER_ERROR' },
+              },
+            ],
+          }) + '\n',
+        );
       });
     });
   });
-
-  describe('throwHttpGraphQLError', () => {
-    it('should add no-cache headers if error is of type PersistedQueryNotSupportedError', () => {
-      try {
-        throwHttpGraphQLError(200, [new PersistedQueryNotSupportedError()]);
-      } catch (err) {
-        expect(err.headers).toEqual({
-          'Content-Type': 'application/json',
-          'Cache-Control': 'private, no-cache, must-revalidate',
-        });
-      }
-    });
-
-    it('should add no-cache headers if error is of type PersistedQueryNotFoundError', () => {
-      try {
-        throwHttpGraphQLError(200, [new PersistedQueryNotFoundError()]);
-      } catch (err) {
-        expect(err.headers).toEqual({
-          'Content-Type': 'application/json',
-          'Cache-Control': 'private, no-cache, must-revalidate',
-        });
-      }
-    });
-
-    it('should not add no-cache headers if error is not a PersistedQuery error', () => {
-      try {
-        throwHttpGraphQLError(200, [new ForbiddenError('401')]);
-      } catch (err) {
-        expect(err.headers).toEqual({ 'Content-Type': 'application/json' });
-      }
-    });
-  });
 });
diff --git a/packages/apollo-server-core/src/index.ts b/packages/apollo-server-core/src/index.ts
@@ -32,7 +32,6 @@ export {
   GraphQLRequestMetrics,
   GraphQLRequestContext,
   ValidationRule,
-  InvalidGraphQLRequestError,
   GraphQLExecutor,
   GraphQLExecutionResult,
 } from 'apollo-server-types';

diff --git a/packages/apollo-server-core/src/plugin/usageReporting/options.ts b/packages/apollo-server-core/src/plugin/usageReporting/options.ts
@@ -1,9 +1,9 @@
 import { GraphQLError, DocumentNode } from 'graphql';
 import {
   GraphQLRequestContextDidResolveOperation,
-  GraphQLRequestContextDidEncounterErrors,
   Logger,
   GraphQLRequestContext,
+  GraphQLRequestContextWillSendResponse,
 } from 'apollo-server-types';
 import type { fetch, RequestAgent } from 'apollo-server-env';
 import type { Trace } from 'apollo-reporting-protobuf';
@@ -63,7 +63,7 @@ export interface ApolloServerPluginUsageReportingOptions<TContext> {
    * phase, which permits tracing based on dynamic properties, e.g., HTTP
    * headers or the `operationName` (when available). Otherwise it will receive
    * the request context in the
-   * [`GraphQLRequestContextDidEncounterError`](https://www.apollographql.com/docs/apollo-server/integrations/plugins/#didencountererrors)
+   * [`GraphQLRequestContextWillSendResponse`](https://www.apollographql.com/docs/apollo-server/integrations/plugins/#willsendresponse)
    * phase:
    *
    * (If you don't want any usage reporting, don't use this plugin; if you are
@@ -92,7 +92,7 @@ export interface ApolloServerPluginUsageReportingOptions<TContext> {
   includeRequest?: (
     request:
       | GraphQLRequestContextDidResolveOperation<TContext>
-      | GraphQLRequestContextDidEncounterErrors<TContext>,
+      | GraphQLRequestContextWillSendResponse<TContext>,
   ) => Promise<boolean>;
   /**
    * By default, this plugin associates client information such as name