From 65fea033fb722110ec238e5e01c3dbd11b10e51e Mon Sep 17 00:00:00 2001 From: Martijn Walraven Date: Thu, 7 Mar 2019 12:28:22 +0100 Subject: [PATCH 1/5] Add pluggable executor to request pipeline --- .../apollo-server-core/src/graphqlOptions.ts | 2 ++ .../apollo-server-core/src/requestPipeline.ts | 32 ++++++++++++------- .../src/requestPipelineAPI.ts | 19 +++++++++-- .../apollo-server-core/src/runHttpQuery.ts | 1 + packages/apollo-server-core/src/types.ts | 1 + 5 files changed, 41 insertions(+), 14 deletions(-) diff --git a/packages/apollo-server-core/src/graphqlOptions.ts b/packages/apollo-server-core/src/graphqlOptions.ts index 7112dbee057..78d22a3b7f4 100644 --- a/packages/apollo-server-core/src/graphqlOptions.ts +++ b/packages/apollo-server-core/src/graphqlOptions.ts @@ -13,6 +13,7 @@ import { DataSource } from 'apollo-datasource'; import { ApolloServerPlugin } from 'apollo-server-plugin-base'; import { GraphQLParseOptions } from 'graphql-tools'; import { ValueOrPromise } from 'apollo-server-env'; +import { GraphQLExecutor } from '../dist/requestPipelineAPI'; /* * GraphQLServerOptions @@ -38,6 +39,7 @@ export interface GraphQLServerOptions< rootValue?: ((parsedQuery: DocumentNode) => TRootValue) | TRootValue; context?: TContext | (() => never); validationRules?: Array<(context: ValidationContext) => any>; + executor?: GraphQLExecutor; formatResponse?: Function; fieldResolver?: GraphQLFieldResolver; debug?: boolean; diff --git a/packages/apollo-server-core/src/requestPipeline.ts b/packages/apollo-server-core/src/requestPipeline.ts index e371e4d9772..566823f0e60 100644 --- a/packages/apollo-server-core/src/requestPipeline.ts +++ b/packages/apollo-server-core/src/requestPipeline.ts @@ -5,9 +5,9 @@ import { DocumentNode, getOperationAST, ExecutionArgs, - ExecutionResult, GraphQLError, GraphQLFormattedError, + ExecutionResult, } from 'graphql'; import * as graphql from 'graphql'; import { @@ -36,6 +36,7 @@ import { GraphQLRequestContext, InvalidGraphQLRequestError, ValidationRule, + GraphQLExecutor, } from '../dist/requestPipelineAPI'; import { ApolloServerPlugin, @@ -73,6 +74,7 @@ export interface GraphQLRequestPipelineConfig { rootValue?: ((document: DocumentNode) => any) | any; validationRules?: ValidationRule[]; + executor?: GraphQLExecutor; fieldResolver?: GraphQLFieldResolver; dataSources?: () => DataSources; @@ -316,11 +318,10 @@ export async function processGraphQLRequest( ); try { - response = (await execute( - requestContext.document, - request.operationName, - request.variables, - )) as GraphQLResponse; + response = await execute(requestContext as WithRequired< + typeof requestContext, + 'document' | 'operation' | 'operationName' + >); executionDidEnd(); } catch (executionError) { executionDidEnd(executionError); @@ -395,10 +396,13 @@ export async function processGraphQLRequest( } async function execute( - document: DocumentNode, - operationName: GraphQLRequest['operationName'], - variables: GraphQLRequest['variables'], + requestContext: WithRequired< + GraphQLRequestContext, + 'document' | 'operationName' | 'operation' + >, ): Promise { + const { request, document } = requestContext; + const executionArgs: ExecutionArgs = { schema: config.schema, document, @@ -407,8 +411,8 @@ export async function processGraphQLRequest( ? config.rootValue(document) : config.rootValue, contextValue: requestContext.context, - variableValues: variables, - operationName, + variableValues: request.variables, + operationName: request.operationName, fieldResolver: config.fieldResolver, }; @@ -417,7 +421,11 @@ export async function processGraphQLRequest( }); try { - return await graphql.execute(executionArgs); + if (config.executor) { + return await config.executor(requestContext); + } else { + return await graphql.execute(executionArgs); + } } finally { executionDidEnd(); } diff --git a/packages/apollo-server-core/src/requestPipelineAPI.ts b/packages/apollo-server-core/src/requestPipelineAPI.ts index 65b4a6b43ee..9dfa5939c12 100644 --- a/packages/apollo-server-core/src/requestPipelineAPI.ts +++ b/packages/apollo-server-core/src/requestPipelineAPI.ts @@ -2,7 +2,12 @@ // circular dependency issues from the `apollo-server-plugin-base` package // depending on the types in it. -import { Request, Response } from 'apollo-server-env'; +import { + Request, + Response, + ValueOrPromise, + WithRequired, +} from 'apollo-server-env'; import { GraphQLSchema, ValidationContext, @@ -10,6 +15,7 @@ import { GraphQLError, OperationDefinitionNode, DocumentNode, + ExecutionResult, } from 'graphql'; import { KeyValueCache } from 'apollo-server-caching'; @@ -27,11 +33,13 @@ export interface GraphQLServiceContext { export interface GraphQLRequest { query?: string; operationName?: string; - variables?: { [name: string]: any }; + variables?: VariableValues; extensions?: Record; http?: Pick; } +export type VariableValues = { [name: string]: any }; + export interface GraphQLResponse { data?: Record; errors?: GraphQLError[]; @@ -72,3 +80,10 @@ export interface GraphQLRequestContext> { export type ValidationRule = (context: ValidationContext) => ASTVisitor; export class InvalidGraphQLRequestError extends Error {} + +export type GraphQLExecutor> = ( + requestContext: WithRequired< + GraphQLRequestContext, + 'document' | 'operationName' | 'operation' + >, +) => ValueOrPromise; diff --git a/packages/apollo-server-core/src/runHttpQuery.ts b/packages/apollo-server-core/src/runHttpQuery.ts index d8f02820d82..2a485ad344d 100644 --- a/packages/apollo-server-core/src/runHttpQuery.ts +++ b/packages/apollo-server-core/src/runHttpQuery.ts @@ -155,6 +155,7 @@ export async function runHttpQuery( rootValue: options.rootValue, context: options.context || {}, validationRules: options.validationRules, + executor: options.executor, fieldResolver: options.fieldResolver, // FIXME: Use proper option types to ensure this diff --git a/packages/apollo-server-core/src/types.ts b/packages/apollo-server-core/src/types.ts index 6f48ca91fc5..d0c391437da 100644 --- a/packages/apollo-server-core/src/types.ts +++ b/packages/apollo-server-core/src/types.ts @@ -56,6 +56,7 @@ type BaseConfig = Pick< | 'debug' | 'rootValue' | 'validationRules' + | 'executor' | 'formatResponse' | 'fieldResolver' | 'tracing' From cc8374acf046fc5a608c7128ebf690e787d7227a Mon Sep 17 00:00:00 2001 From: Martijn Walraven Date: Fri, 8 Mar 2019 13:36:48 +0100 Subject: [PATCH 2/5] Export request pipeline API types from `apollo-server-core` --- packages/apollo-server-core/src/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/apollo-server-core/src/index.ts b/packages/apollo-server-core/src/index.ts index dd6015c4213..b77f89cf680 100644 --- a/packages/apollo-server-core/src/index.ts +++ b/packages/apollo-server-core/src/index.ts @@ -31,6 +31,7 @@ export { // ApolloServer Base class export { ApolloServerBase } from './ApolloServer'; export * from './types'; +export * from './requestPipelineAPI'; // This currently provides the ability to have syntax highlighting as well as // consistency between client and server gql tags From d60c84da3aa654f118d086b596bcb1970acebd66 Mon Sep 17 00:00:00 2001 From: Martijn Walraven Date: Wed, 13 Mar 2019 21:33:51 +0100 Subject: [PATCH 3/5] Change `errors` in `GraphQLResponse` to formatted errors This adjusts the request pipeline to format errors before calling `willSendResponse`, and adds a `didEncounterErrors` hook to give the Engine reporting extension access to the raw errors. --- .../apollo-engine-reporting/src/extension.ts | 9 +---- .../apollo-server-core/src/ApolloServer.ts | 13 ------- packages/apollo-server-core/src/formatters.ts | 35 ----------------- .../apollo-server-core/src/requestPipeline.ts | 38 +++++++++++++++---- .../src/requestPipelineAPI.ts | 4 +- packages/apollo-server-errors/src/index.ts | 2 +- .../src/ApolloServer.ts | 6 +-- packages/graphql-extensions/src/index.ts | 9 +++++ 8 files changed, 47 insertions(+), 69 deletions(-) delete mode 100644 packages/apollo-server-core/src/formatters.ts diff --git a/packages/apollo-engine-reporting/src/extension.ts b/packages/apollo-engine-reporting/src/extension.ts index cfa4bccb051..b8ccd37e364 100644 --- a/packages/apollo-engine-reporting/src/extension.ts +++ b/packages/apollo-engine-reporting/src/extension.ts @@ -8,11 +8,7 @@ import { ExecutionArgs, GraphQLError, } from 'graphql'; -import { - GraphQLExtension, - GraphQLResponse, - EndHandler, -} from 'graphql-extensions'; +import { GraphQLExtension, EndHandler } from 'graphql-extensions'; import { Trace, google } from 'apollo-engine-reporting-protobuf'; import { EngineReportingOptions, GenerateClientInfo } from './agent'; @@ -279,8 +275,7 @@ export class EngineReportingExtension }; } - public willSendResponse(o: { graphqlResponse: GraphQLResponse }) { - const { errors } = o.graphqlResponse; + public didEncounterErrors(errors: GraphQLError[]) { if (errors) { errors.forEach((error: GraphQLError) => { // By default, put errors on the root node. diff --git a/packages/apollo-server-core/src/ApolloServer.ts b/packages/apollo-server-core/src/ApolloServer.ts index 3b3dc379b71..e8304035ded 100644 --- a/packages/apollo-server-core/src/ApolloServer.ts +++ b/packages/apollo-server-core/src/ApolloServer.ts @@ -43,8 +43,6 @@ import { PluginDefinition, } from './types'; -import { FormatErrorExtension } from './formatters'; - import { gql } from './index'; import { @@ -331,17 +329,6 @@ export class ApolloServerBase { // or cacheControl. this.extensions = []; - const debugDefault = - process.env.NODE_ENV !== 'production' && process.env.NODE_ENV !== 'test'; - const debug = - requestOptions.debug !== undefined ? requestOptions.debug : debugDefault; - - // Error formatting should happen after the engine reporting agent, so that - // engine gets the unmasked errors if necessary - this.extensions.push( - () => new FormatErrorExtension(requestOptions.formatError, debug), - ); - // In an effort to avoid over-exposing the API key itself, extract the // service ID from the API key for plugins which only needs service ID. // The truthyness of this value can also be used in other forks of logic diff --git a/packages/apollo-server-core/src/formatters.ts b/packages/apollo-server-core/src/formatters.ts deleted file mode 100644 index 1abd83cf0e0..00000000000 --- a/packages/apollo-server-core/src/formatters.ts +++ /dev/null @@ -1,35 +0,0 @@ -import { GraphQLExtension, GraphQLResponse } from 'graphql-extensions'; -import { formatApolloErrors } from 'apollo-server-errors'; -import { GraphQLError, GraphQLFormattedError } from 'graphql'; - -export class FormatErrorExtension extends GraphQLExtension { - private formatError?: (error: GraphQLError) => GraphQLFormattedError; - private debug: boolean; - - public constructor( - formatError?: (error: GraphQLError) => GraphQLFormattedError, - debug: boolean = false, - ) { - super(); - this.formatError = formatError; - this.debug = debug; - } - - public willSendResponse(o: { - graphqlResponse: GraphQLResponse; - context: TContext; - }): void | { graphqlResponse: GraphQLResponse; context: TContext } { - if (o.graphqlResponse.errors) { - return { - ...o, - graphqlResponse: { - ...o.graphqlResponse, - errors: formatApolloErrors(o.graphqlResponse.errors, { - formatter: this.formatError, - debug: this.debug, - }), - }, - }; - } - } -} diff --git a/packages/apollo-server-core/src/requestPipeline.ts b/packages/apollo-server-core/src/requestPipeline.ts index 566823f0e60..8ec7cbce689 100644 --- a/packages/apollo-server-core/src/requestPipeline.ts +++ b/packages/apollo-server-core/src/requestPipeline.ts @@ -29,6 +29,7 @@ import { ValidationError, PersistedQueryNotSupportedError, PersistedQueryNotFoundError, + formatApolloErrors, } from 'apollo-server-errors'; import { GraphQLRequest, @@ -318,10 +319,20 @@ export async function processGraphQLRequest( ); try { - response = await execute(requestContext as WithRequired< + const result = await execute(requestContext as WithRequired< typeof requestContext, 'document' | 'operation' | 'operationName' >); + + if (result.errors) { + extensionStack.didEncounterErrors(result.errors); + } + + response = { + ...result, + errors: result.errors ? formatErrors(result.errors) : undefined, + }; + executionDidEnd(); } catch (executionError) { executionDidEnd(executionError); @@ -341,7 +352,7 @@ export async function processGraphQLRequest( } else { requestContext.overallCachePolicy = cacheControlExtension.computeOverallCachePolicy(); } - } + const formattedExtensions = extensionStack.format(); if (Object.keys(formattedExtensions).length > 0) { @@ -462,17 +473,28 @@ export async function processGraphQLRequest( : [errorOrErrors]; return sendResponse({ - errors: errors.map(err => - fromGraphQLError( - err, - errorClass && { - errorClass, - }, + errors: formatErrors( + errors.map(err => + fromGraphQLError( + err, + errorClass && { + errorClass, + }, + ), ), ), }); } + function formatErrors( + errors: ReadonlyArray, + ): ReadonlyArray { + return formatApolloErrors(errors, { + formatter: config.formatError, + debug: requestContext.debug, + }); + } + function initializeRequestListenerDispatcher(): Dispatcher< GraphQLRequestListener > { diff --git a/packages/apollo-server-core/src/requestPipelineAPI.ts b/packages/apollo-server-core/src/requestPipelineAPI.ts index 9dfa5939c12..bb497244dcb 100644 --- a/packages/apollo-server-core/src/requestPipelineAPI.ts +++ b/packages/apollo-server-core/src/requestPipelineAPI.ts @@ -12,7 +12,7 @@ import { GraphQLSchema, ValidationContext, ASTVisitor, - GraphQLError, + GraphQLFormattedError, OperationDefinitionNode, DocumentNode, ExecutionResult, @@ -42,7 +42,7 @@ export type VariableValues = { [name: string]: any }; export interface GraphQLResponse { data?: Record; - errors?: GraphQLError[]; + errors?: ReadonlyArray; extensions?: Record; http?: Pick; } diff --git a/packages/apollo-server-errors/src/index.ts b/packages/apollo-server-errors/src/index.ts index f330df8a7fe..b29f060dba5 100644 --- a/packages/apollo-server-errors/src/index.ts +++ b/packages/apollo-server-errors/src/index.ts @@ -211,7 +211,7 @@ export class UserInputError extends ApolloError { } export function formatApolloErrors( - errors: Array, + errors: ReadonlyArray, options?: { formatter?: (error: GraphQLError) => GraphQLFormattedError; debug?: boolean; diff --git a/packages/apollo-server-integration-testsuite/src/ApolloServer.ts b/packages/apollo-server-integration-testsuite/src/ApolloServer.ts index 2238fd29798..c2b2e5d73a1 100644 --- a/packages/apollo-server-integration-testsuite/src/ApolloServer.ts +++ b/packages/apollo-server-integration-testsuite/src/ApolloServer.ts @@ -625,9 +625,9 @@ export function testApolloServer( context: TContext; }) { expect(o.graphqlResponse.errors.length).toEqual(1); - // formatError should be called after extensions - expect(formatError).not.toBeCalled(); - // validationRules should be called before extensions + // formatError should be called before willSendResponse + expect(formatError).toHaveBeenCalledTimes(1); + // validationRule should be called before willSendResponse expect(validationRule).toHaveBeenCalledTimes(1); willSendResponseInExtension(); } diff --git a/packages/graphql-extensions/src/index.ts b/packages/graphql-extensions/src/index.ts index 68d6a688426..ad1c79f241e 100644 --- a/packages/graphql-extensions/src/index.ts +++ b/packages/graphql-extensions/src/index.ts @@ -10,6 +10,7 @@ import { DocumentNode, ResponsePath, FieldNode, + GraphQLError, } from 'graphql'; import { Request } from 'apollo-server-env'; @@ -52,6 +53,7 @@ export class GraphQLExtension { public didResolveOperation?(o: { requestContext: GraphQLRequestContext; }): void; + public didEncounterErrors?(errors: ReadonlyArray): void; public willSendResponse?(o: { graphqlResponse: GraphQLResponse; @@ -112,12 +114,19 @@ export class GraphQLExtensionStack { ); } +<<<<<<< HEAD public didResolveOperation(o: { requestContext: GraphQLRequestContext; }) { this.extensions.forEach(extension => { if (extension.didResolveOperation) { extension.didResolveOperation(o); +======= + public didEncounterErrors(errors: ReadonlyArray) { + this.extensions.forEach(extension => { + if (extension.didEncounterErrors) { + extension.didEncounterErrors(errors); +>>>>>>> Change `errors` in `GraphQLResponse` to formatted errors } }); } From abc16921c1647f2b873e2cdcb9e6605d23f04b7f Mon Sep 17 00:00:00 2001 From: Martijn Walraven Date: Fri, 22 Mar 2019 19:34:56 +0100 Subject: [PATCH 4/5] Introduce `GraphQLExecutionResult` type --- packages/apollo-server-core/src/requestPipeline.ts | 4 ++-- packages/apollo-server-core/src/requestPipelineAPI.ts | 10 ++++++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/apollo-server-core/src/requestPipeline.ts b/packages/apollo-server-core/src/requestPipeline.ts index 8ec7cbce689..a9fc95d486b 100644 --- a/packages/apollo-server-core/src/requestPipeline.ts +++ b/packages/apollo-server-core/src/requestPipeline.ts @@ -7,7 +7,6 @@ import { ExecutionArgs, GraphQLError, GraphQLFormattedError, - ExecutionResult, } from 'graphql'; import * as graphql from 'graphql'; import { @@ -38,6 +37,7 @@ import { InvalidGraphQLRequestError, ValidationRule, GraphQLExecutor, + GraphQLExecutionResult, } from '../dist/requestPipelineAPI'; import { ApolloServerPlugin, @@ -411,7 +411,7 @@ export async function processGraphQLRequest( GraphQLRequestContext, 'document' | 'operationName' | 'operation' >, - ): Promise { + ): Promise { const { request, document } = requestContext; const executionArgs: ExecutionArgs = { diff --git a/packages/apollo-server-core/src/requestPipelineAPI.ts b/packages/apollo-server-core/src/requestPipelineAPI.ts index bb497244dcb..359d0007b4d 100644 --- a/packages/apollo-server-core/src/requestPipelineAPI.ts +++ b/packages/apollo-server-core/src/requestPipelineAPI.ts @@ -15,7 +15,7 @@ import { GraphQLFormattedError, OperationDefinitionNode, DocumentNode, - ExecutionResult, + GraphQLError, } from 'graphql'; import { KeyValueCache } from 'apollo-server-caching'; @@ -86,4 +86,10 @@ export type GraphQLExecutor> = ( GraphQLRequestContext, 'document' | 'operationName' | 'operation' >, -) => ValueOrPromise; +) => ValueOrPromise; + +export type GraphQLExecutionResult = { + data?: Record; + errors?: ReadonlyArray; + extensions?: Record; +}; From 7cdc8c0c56d558185b21e70e10e3ee1aa7045cc3 Mon Sep 17 00:00:00 2001 From: James Baxley Date: Sat, 13 Apr 2019 22:43:31 -0400 Subject: [PATCH 5/5] fix merge conflicts --- package-lock.json | 8 +++++++- packages/apollo-server-core/src/requestPipeline.ts | 2 +- packages/graphql-extensions/src/index.ts | 7 +++---- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/package-lock.json b/package-lock.json index e8bc689a7da..37359e290c7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3813,7 +3813,13 @@ "version": "file:packages/apollo-server-plugin-base" }, "apollo-server-plugin-response-cache": { - "version": "file:packages/apollo-server-plugin-response-cache" + "version": "file:packages/apollo-server-plugin-response-cache", + "requires": { + "apollo-cache-control": "file:packages/apollo-cache-control", + "apollo-server-caching": "file:packages/apollo-server-caching", + "apollo-server-env": "file:packages/apollo-server-env", + "apollo-server-plugin-base": "file:packages/apollo-server-plugin-base" + } }, "apollo-server-testing": { "version": "file:packages/apollo-server-testing", diff --git a/packages/apollo-server-core/src/requestPipeline.ts b/packages/apollo-server-core/src/requestPipeline.ts index a9fc95d486b..27e10daa3b3 100644 --- a/packages/apollo-server-core/src/requestPipeline.ts +++ b/packages/apollo-server-core/src/requestPipeline.ts @@ -352,7 +352,7 @@ export async function processGraphQLRequest( } else { requestContext.overallCachePolicy = cacheControlExtension.computeOverallCachePolicy(); } - + } const formattedExtensions = extensionStack.format(); if (Object.keys(formattedExtensions).length > 0) { diff --git a/packages/graphql-extensions/src/index.ts b/packages/graphql-extensions/src/index.ts index ad1c79f241e..c1b051d9439 100644 --- a/packages/graphql-extensions/src/index.ts +++ b/packages/graphql-extensions/src/index.ts @@ -14,7 +14,6 @@ import { } from 'graphql'; import { Request } from 'apollo-server-env'; -export { Request } from 'apollo-server-env'; import { GraphQLResponse, @@ -114,19 +113,19 @@ export class GraphQLExtensionStack { ); } -<<<<<<< HEAD public didResolveOperation(o: { requestContext: GraphQLRequestContext; }) { this.extensions.forEach(extension => { if (extension.didResolveOperation) { extension.didResolveOperation(o); -======= + } + }); + } public didEncounterErrors(errors: ReadonlyArray) { this.extensions.forEach(extension => { if (extension.didEncounterErrors) { extension.didEncounterErrors(errors); ->>>>>>> Change `errors` in `GraphQLResponse` to formatted errors } }); }