From 2ef50aeed9adda8cf7e5cbb355f12ad986673ef4 Mon Sep 17 00:00:00 2001 From: Antoine Sauvage Date: Mon, 10 Dec 2018 15:02:24 +0100 Subject: [PATCH 1/4] WIP WIP --- packages/apollo-cache-control/src/index.ts | 43 ++++++++--- .../apollo-engine-reporting/src/extension.ts | 71 ++++++++++++++----- .../apollo-server-core/src/ApolloServer.ts | 4 +- packages/apollo-server-core/src/formatters.ts | 60 +++++++++++++--- .../apollo-server-core/src/requestPipeline.ts | 4 +- .../apollo-server-core/src/runHttpQuery.ts | 20 +++--- packages/apollo-server-errors/src/index.ts | 54 +++++++++----- packages/graphql-extensions/src/index.ts | 45 ++++++++---- 8 files changed, 221 insertions(+), 80 deletions(-) diff --git a/packages/apollo-cache-control/src/index.ts b/packages/apollo-cache-control/src/index.ts index db145f7a15f..ca642150b3f 100644 --- a/packages/apollo-cache-control/src/index.ts +++ b/packages/apollo-cache-control/src/index.ts @@ -9,6 +9,7 @@ import { } from 'graphql'; import { GraphQLExtension, GraphQLResponse } from 'graphql-extensions'; +import {isPromise} from "apollo-server-errors"; export interface CacheControlFormat { version: 1; @@ -136,18 +137,38 @@ export class CacheControlExtension ]; } - public willSendResponse?(o: { graphqlResponse: GraphQLResponse }) { - if (this.options.calculateHttpHeaders && o.graphqlResponse.http) { - const overallCachePolicy = this.computeOverallCachePolicy(); - - if (overallCachePolicy) { - o.graphqlResponse.http.headers.set( - 'Cache-Control', - `max-age=${ - overallCachePolicy.maxAge - }, ${overallCachePolicy.scope.toLowerCase()}`, - ); + public willSendResponse?(o: { graphqlResponse: GraphQLResponse } | Promise<{ graphqlResponse: GraphQLResponse }>) { + if(!isPromise(o)) { + if (this.options.calculateHttpHeaders && o.graphqlResponse.http) { + const overallCachePolicy = this.computeOverallCachePolicy(); + + if (overallCachePolicy) { + o.graphqlResponse.http.headers.set( + 'Cache-Control', + `max-age=${ + overallCachePolicy.maxAge + }, ${overallCachePolicy.scope.toLowerCase()}`, + ); + } } + return + } + else { + return o.then(p => { + if (this.options.calculateHttpHeaders && p.graphqlResponse.http) { + const overallCachePolicy = this.computeOverallCachePolicy(); + + if (overallCachePolicy) { + p.graphqlResponse.http.headers.set( + 'Cache-Control', + `max-age=${ + overallCachePolicy.maxAge + }, ${overallCachePolicy.scope.toLowerCase()}`, + ); + } + } + return + }) } } diff --git a/packages/apollo-engine-reporting/src/extension.ts b/packages/apollo-engine-reporting/src/extension.ts index 37ca3adfcc1..3618d6d9759 100644 --- a/packages/apollo-engine-reporting/src/extension.ts +++ b/packages/apollo-engine-reporting/src/extension.ts @@ -18,6 +18,7 @@ import { Trace, google } from 'apollo-engine-reporting-protobuf'; import { EngineReportingOptions, GenerateClientInfo } from './agent'; import { defaultSignature } from './signature'; import { GraphQLRequestContext } from 'apollo-server-core/dist/requestPipelineAPI'; +import {isPromise} from "apollo-server-errors"; const clientNameHeaderKey = 'apollographql-client-name'; const clientReferenceIdHeaderKey = 'apollographql-client-reference-id'; @@ -275,32 +276,66 @@ export class EngineReportingExtension }; } - public willSendResponse(o: { graphqlResponse: GraphQLResponse }) { - const { errors } = o.graphqlResponse; - if (errors) { - errors.forEach((error: GraphQLError) => { - // By default, put errors on the root node. - let node = this.nodes.get(''); - if (error.path) { - const specificNode = this.nodes.get(error.path.join('.')); - if (specificNode) { - node = specificNode; + public willSendResponse(o: { graphqlResponse: GraphQLResponse } | Promise<{ graphqlResponse: GraphQLResponse }>) { + if (!isPromise(o)) { + const {errors} = o.graphqlResponse; + if (errors) { + errors.forEach((error: GraphQLError) => { + // By default, put errors on the root node. + let node = this.nodes.get(''); + if (error.path) { + const specificNode = this.nodes.get(error.path.join('.')); + if (specificNode) { + node = specificNode; + } } - } - // Always send the trace errors, so that the UI acknowledges that there is an error. - const errorInfo = this.options.maskErrorDetails - ? { message: '' } - : { + // Always send the trace errors, so that the UI acknowledges that there is an error. + const errorInfo = this.options.maskErrorDetails + ? {message: ''} + : { message: error.message, location: (error.locations || []).map( - ({ line, column }) => new Trace.Location({ line, column }), + ({line, column}) => new Trace.Location({line, column}), ), json: JSON.stringify(error), }; - node!.error!.push(new Trace.Error(errorInfo)); - }); + node!.error!.push(new Trace.Error(errorInfo)); + }); + } + return + } + else { + return o.then(p => { + const {errors} = p.graphqlResponse; + if (errors) { + errors.forEach((error: GraphQLError) => { + // By default, put errors on the root node. + let node = this.nodes.get(''); + if (error.path) { + const specificNode = this.nodes.get(error.path.join('.')); + if (specificNode) { + node = specificNode; + } + } + + // Always send the trace errors, so that the UI acknowledges that there is an error. + const errorInfo = this.options.maskErrorDetails + ? {message: ''} + : { + message: error.message, + location: (error.locations || []).map( + ({line, column}) => new Trace.Location({line, column}), + ), + json: JSON.stringify(error), + }; + + node!.error!.push(new Trace.Error(errorInfo)); + }); + } + return + }) } } diff --git a/packages/apollo-server-core/src/ApolloServer.ts b/packages/apollo-server-core/src/ApolloServer.ts index 7b715ba005b..ccccfe992ca 100644 --- a/packages/apollo-server-core/src/ApolloServer.ts +++ b/packages/apollo-server-core/src/ApolloServer.ts @@ -438,10 +438,10 @@ export class ApolloServerBase { ? await this.context({ connection, payload: message.payload }) : context; } catch (e) { - throw formatApolloErrors([e], { + throw (await formatApolloErrors([e], { formatter: this.requestOptions.formatError, debug: this.requestOptions.debug, - })[0]; + }))[0]; } return { ...connection, context }; diff --git a/packages/apollo-server-core/src/formatters.ts b/packages/apollo-server-core/src/formatters.ts index 2410f74aee2..e2013aa74c1 100644 --- a/packages/apollo-server-core/src/formatters.ts +++ b/packages/apollo-server-core/src/formatters.ts @@ -1,5 +1,10 @@ import { GraphQLExtension, GraphQLResponse } from 'graphql-extensions'; -import { formatApolloErrors } from 'apollo-server-errors'; +import {formatApolloErrors} from 'apollo-server-errors'; + + +function isPromise(x: any): x is Promise { + return x && typeof x.then === 'function'; +} export class FormatErrorExtension extends GraphQLExtension { private formatError?: Function; @@ -11,21 +16,60 @@ export class FormatErrorExtension extends GraphQLExtension { this.debug = debug; } - public willSendResponse(o: { + public willSendResponse(o: Promise<{ + graphqlResponse: GraphQLResponse; + context: TContext; + }> | { graphqlResponse: GraphQLResponse; context: TContext; - }): void | { graphqlResponse: GraphQLResponse; context: TContext } { - if (o.graphqlResponse.errors) { + }): Promise<{ graphqlResponse: GraphQLResponse; context: TContext } | void> | + { graphqlResponse: GraphQLResponse; context: TContext } | void { + + + if (isPromise(o)) { + return this.asyncWillSendResponse(o) + } + else if(o.graphqlResponse.errors){ + let formattedErrors = formatApolloErrors(o.graphqlResponse.errors, { + formatter: this.formatError, + debug: this.debug, + }); + if(isPromise(formattedErrors)){ + return this.asyncWillSendResponse(o) + } + else if(o) { + return ({ + ...o, + graphqlResponse: { + ...o.graphqlResponse, + errors: formattedErrors + }, + }) + } + } + return o + } + + + public async asyncWillSendResponse(o: Promise<{ + graphqlResponse: GraphQLResponse; + context: TContext; + }> | { graphqlResponse: GraphQLResponse; context: TContext }): Promise<{ graphqlResponse: GraphQLResponse; context: TContext } | void> + { + let p = await o + if (p.graphqlResponse.errors) { return { - ...o, - graphqlResponse: { - ...o.graphqlResponse, - errors: formatApolloErrors(o.graphqlResponse.errors, { + ...p, + graphqlResponse: { + ...p.graphqlResponse, + errors: await formatApolloErrors(p.graphqlResponse.errors, { formatter: this.formatError, debug: this.debug, }), }, }; } + return o } } + diff --git a/packages/apollo-server-core/src/requestPipeline.ts b/packages/apollo-server-core/src/requestPipeline.ts index 7c3d672eeee..797f773cdb8 100644 --- a/packages/apollo-server-core/src/requestPipeline.ts +++ b/packages/apollo-server-core/src/requestPipeline.ts @@ -330,7 +330,7 @@ export async function processGraphQLRequest( ): Promise { // We override errors, data, and extensions with the passed in response, // but keep other properties (like http) - requestContext.response = extensionStack.willSendResponse({ + requestContext.response = (await extensionStack.willSendResponse({ graphqlResponse: { ...requestContext.response, errors: response.errors, @@ -338,7 +338,7 @@ export async function processGraphQLRequest( extensions: response.extensions, }, context: requestContext.context, - }).graphqlResponse; + })).graphqlResponse; await dispatcher.invokeHookAsync( 'willSendResponse', requestContext as WithRequired, diff --git a/packages/apollo-server-core/src/runHttpQuery.ts b/packages/apollo-server-core/src/runHttpQuery.ts index 977593bcdd3..ccb55751551 100644 --- a/packages/apollo-server-core/src/runHttpQuery.ts +++ b/packages/apollo-server-core/src/runHttpQuery.ts @@ -69,16 +69,16 @@ export class HttpQueryError extends Error { /** * If options is specified, then the errors array will be formatted */ -function throwHttpGraphQLError( +async function throwHttpGraphQLError( statusCode: number, errors: Array, options?: Pick, -): never { +): Promise | never { throw new HttpQueryError( statusCode, prettyJSONStringify({ errors: options - ? formatApolloErrors(errors, { + ? await formatApolloErrors(errors, { debug: options.debug, formatter: options.formatError, }) @@ -110,7 +110,7 @@ export async function runHttpQuery( if (!debugDefault) { e.warning = `To remove the stacktrace, set the NODE_ENV environment variable to production if the options creation can fail`; } - return throwHttpGraphQLError(500, [e], { debug: debugDefault }); + return await throwHttpGraphQLError(500, [e], { debug: debugDefault }); } if (options.debug === undefined) { options.debug = debugDefault; @@ -135,9 +135,9 @@ export async function runHttpQuery( e.extensions.code && e.extensions.code !== 'INTERNAL_SERVER_ERROR' ) { - return throwHttpGraphQLError(400, [e], options); + return await throwHttpGraphQLError(400, [e], options); } else { - return throwHttpGraphQLError(500, [e], options); + return await throwHttpGraphQLError(500, [e], options); } } } @@ -265,7 +265,7 @@ export async function processHTTPRequest( // A batch can contain another query that returns data, // so we don't error out the entire request with an HttpError return { - errors: formatApolloErrors([error], options), + errors: await formatApolloErrors([error], options), }; } }), @@ -284,7 +284,7 @@ export async function processHTTPRequest( // doesn't reach GraphQL execution if (response.errors && typeof response.data === 'undefined') { // don't include options, since the errors have already been formatted - return throwHttpGraphQLError(400, response.errors as any); + return await throwHttpGraphQLError(400, response.errors as any); } if (response.http) { @@ -301,7 +301,7 @@ export async function processHTTPRequest( error instanceof PersistedQueryNotSupportedError || error instanceof PersistedQueryNotFoundError ) { - return throwHttpGraphQLError(200, [error], options); + return await throwHttpGraphQLError(200, [error], options); } else { throw error; } @@ -311,7 +311,7 @@ export async function processHTTPRequest( if (error instanceof HttpQueryError) { throw error; } - return throwHttpGraphQLError(500, [error], options); + return await throwHttpGraphQLError(500, [error], options); } responseInit.headers!['Content-Length'] = Buffer.byteLength( diff --git a/packages/apollo-server-errors/src/index.ts b/packages/apollo-server-errors/src/index.ts index 17c7dd64404..299be704efc 100644 --- a/packages/apollo-server-errors/src/index.ts +++ b/packages/apollo-server-errors/src/index.ts @@ -1,5 +1,9 @@ import { GraphQLError } from 'graphql'; +export function isPromise(x: any): x is Promise { + return x && typeof x.then === 'function'; +} + export class ApolloError extends Error implements GraphQLError { public extensions: Record; readonly name; @@ -216,7 +220,7 @@ export function formatApolloErrors( formatter?: Function; debug?: boolean; }, -): Array { +): Promise> | Array { if (!options) { return errors.map(error => enrichError(error)); } @@ -247,20 +251,38 @@ export function formatApolloErrors( if (!formatter) { return enrichedErrors; } - - return enrichedErrors.map(error => { - try { - return formatter(error); - } catch (err) { - if (debug) { - return enrichError(err, debug); - } else { - // obscure error - const newError = fromGraphQLError( - new GraphQLError('Internal server error'), - ); - return enrichError(newError, debug); + if(isPromise(formatter)) { + return Promise.all(enrichedErrors.map(async error => { + try { + return await formatter(error); + } catch (err) { + if (debug) { + return enrichError(err, debug); + } else { + // obscure error + const newError = fromGraphQLError( + new GraphQLError('Internal server error'), + ); + return enrichError(newError, debug); + } } - } - }) as Array; + })); + } + else { + return enrichedErrors.map(error => { + try { + return formatter(error); + } catch (err) { + if (debug) { + return enrichError(err, debug); + } else { + // obscure error + const newError = fromGraphQLError( + new GraphQLError('Internal server error'), + ); + return enrichError(newError, debug); + } + } + }) + } } diff --git a/packages/graphql-extensions/src/index.ts b/packages/graphql-extensions/src/index.ts index 467e4017d4e..0a9486377d7 100644 --- a/packages/graphql-extensions/src/index.ts +++ b/packages/graphql-extensions/src/index.ts @@ -49,10 +49,14 @@ export class GraphQLExtension { executionArgs: ExecutionArgs; }): EndHandler | void; - public willSendResponse?(o: { + public willSendResponse?(o: Promise<{ graphqlResponse: GraphQLResponse; context: TContext; - }): void | { graphqlResponse: GraphQLResponse; context: TContext }; + }> | { + graphqlResponse: GraphQLResponse; + context: TContext; + }): Promise<{ graphqlResponse: GraphQLResponse; context: TContext } | void> | + { graphqlResponse: GraphQLResponse; context: TContext } | void public willResolveField?( source: any, @@ -108,21 +112,36 @@ export class GraphQLExtensionStack { ); } - public willSendResponse(o: { + public willSendResponse(o: Promise<{ + graphqlResponse: GraphQLResponse; + context: TContext; + }> | { + graphqlResponse: GraphQLResponse; + context: TContext; + }): Promise<{ graphqlResponse: GraphQLResponse; context: TContext }> | { graphqlResponse: GraphQLResponse; context: TContext; - }): { graphqlResponse: GraphQLResponse; context: TContext } { - let reference = o; + } { + // Reverse the array, since this is functions as an end handler - [...this.extensions].reverse().forEach(extension => { - if (extension.willSendResponse) { - const result = extension.willSendResponse(reference); - if (result) { - reference = result; + + return [...this.extensions].reverse().reduce((promiseChain, currentTask) => { + if(currentTask.willSendResponse) { + let possiblyAPromise = currentTask.willSendResponse(promiseChain) + if(possiblyAPromise && isPromise(possiblyAPromise)){ + return possiblyAPromise.then(res => res || promiseChain) + } + else if(possiblyAPromise) { + return possiblyAPromise + } + else { + return promiseChain } } - }); - return reference; + else { + return promiseChain + } + }, o); } public willResolveField( @@ -282,7 +301,7 @@ function wrapField(field: GraphQLField): void { }; } -function isPromise(x: any): boolean { +function isPromise(x: any): x is Promise { return x && typeof x.then === 'function'; } From 994d3bc7b12a42b7798c6e4a4473fe5410448896 Mon Sep 17 00:00:00 2001 From: Antoine Sauvage Date: Mon, 10 Dec 2018 17:37:13 +0100 Subject: [PATCH 2/4] writing async test --- .../src/ApolloServer.ts | 118 ++++++++++++++++++ 1 file changed, 118 insertions(+) diff --git a/packages/apollo-server-integration-testsuite/src/ApolloServer.ts b/packages/apollo-server-integration-testsuite/src/ApolloServer.ts index 4cd7156b3b5..b189cb20ff7 100644 --- a/packages/apollo-server-integration-testsuite/src/ApolloServer.ts +++ b/packages/apollo-server-integration-testsuite/src/ApolloServer.ts @@ -635,6 +635,124 @@ export function testApolloServer( expect(trace.root!.child![0].error![0].message).toMatch(/nope/); expect(trace.root!.child![0].error![0].message).not.toMatch(/masked/); }); + + it('validation > engine > extensions > asyncFormatError', async () => { + const throwError = jest.fn(() => { + throw new Error('nope'); + }); + + const validationRule = jest.fn( () => { + // formatError should be called after validation + expect(formatError).not.toBeCalled(); + // extension should be called after validation + expect(willSendResponseInExtension).not.toBeCalled(); + return true; + }); + + const willSendResponseInExtension = jest.fn(); + + const formatError = jest.fn(async error => { + try { + await new Promise(resolve => setTimeout(resolve, 500)) + expect(error).toBeInstanceOf(Error); + // extension should be called before formatError + expect(willSendResponseInExtension).toHaveBeenCalledTimes(1); + // validationRules should be called before formatError + expect(validationRule).toHaveBeenCalledTimes(1); + } finally { + error.message = 'masked'; + return error; + } + }); + + class Extension extends GraphQLExtension { + willSendResponse(o: { + graphqlResponse: GraphQLResponse; + 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 + expect(validationRule).toHaveBeenCalledTimes(1); + willSendResponseInExtension(); + } + } + + let engineServerDidStart: Promise; + + const didReceiveTrace = new Promise(resolve => { + engineServerDidStart = startEngineServer({ + check: (req, res) => { + const report = FullTracesReport.decode(req.body); + const header = report.header; + expect(header.schemaTag).toEqual(''); + expect(header.schemaHash).toBeDefined(); + const trace = Object.values(report.tracesPerQuery)[0].trace[0]; + resolve(trace); + res.end(); + }, + }); + }); + + await engineServerDidStart; + + const { url: uri } = await createApolloServer({ + typeDefs: gql` + type Query { + error: String + } + `, + resolvers: { + Query: { + error: () => { + throwError(); + }, + }, + }, + validationRules: [validationRule], + extensions: [() => new Extension()], + engine: { + endpointUrl: `http://localhost:${ + (engineServer.address() as net.AddressInfo).port + }`, + apiKey: 'service:my-app:secret', + maxUncompressedReportSize: 1, + generateClientInfo: () => ({ + clientName: 'testing', + clientReferenceId: '1234', + clientVersion: 'v1.0.1', + }), + }, + formatError, + debug: true, + }); + + const apolloFetch = createApolloFetch({ uri }); + + const result = await apolloFetch({ + query: `{error}`, + }); + expect(result.data).toEqual({ + error: null, + }); + expect(result.errors).toBeDefined(); + expect(result.errors[0].message).toEqual('masked'); + + expect(validationRule).toHaveBeenCalledTimes(1); + expect(throwError).toHaveBeenCalledTimes(1); + expect(formatError).toHaveBeenCalledTimes(1); + expect(willSendResponseInExtension).toHaveBeenCalledTimes(1); + + const trace = await didReceiveTrace; + + expect(trace.clientReferenceId).toMatch(/1234/); + expect(trace.clientName).toMatch(/testing/); + expect(trace.clientVersion).toEqual('v1.0.1'); + + expect(trace.root!.child![0].error![0].message).toMatch(/nope/); + expect(trace.root!.child![0].error![0].message).not.toMatch(/masked/); + }); }); it('errors thrown in extensions call formatError and are wrapped', async () => { From 606f605b39569460f17bfea0d71a828d0083a42f Mon Sep 17 00:00:00 2001 From: Antoine Sauvage Date: Mon, 10 Dec 2018 17:59:11 +0100 Subject: [PATCH 3/4] linter --- packages/apollo-cache-control/src/index.ts | 23 +++--- .../apollo-engine-reporting/src/extension.ts | 49 ++++++------ packages/apollo-server-core/src/formatters.ts | 76 +++++++++--------- packages/apollo-server-errors/src/index.ts | 37 ++++----- .../src/ApolloServer.ts | 12 +-- packages/graphql-extensions/src/index.ts | 80 +++++++++++-------- 6 files changed, 148 insertions(+), 129 deletions(-) diff --git a/packages/apollo-cache-control/src/index.ts b/packages/apollo-cache-control/src/index.ts index ca642150b3f..958c9728d3b 100644 --- a/packages/apollo-cache-control/src/index.ts +++ b/packages/apollo-cache-control/src/index.ts @@ -9,7 +9,7 @@ import { } from 'graphql'; import { GraphQLExtension, GraphQLResponse } from 'graphql-extensions'; -import {isPromise} from "apollo-server-errors"; +import { isPromise } from 'apollo-server-errors'; export interface CacheControlFormat { version: 1; @@ -137,8 +137,12 @@ export class CacheControlExtension ]; } - public willSendResponse?(o: { graphqlResponse: GraphQLResponse } | Promise<{ graphqlResponse: GraphQLResponse }>) { - if(!isPromise(o)) { + public willSendResponse?( + o: + | { graphqlResponse: GraphQLResponse } + | Promise<{ graphqlResponse: GraphQLResponse }>, + ) { + if (!isPromise(o)) { if (this.options.calculateHttpHeaders && o.graphqlResponse.http) { const overallCachePolicy = this.computeOverallCachePolicy(); @@ -147,13 +151,12 @@ export class CacheControlExtension 'Cache-Control', `max-age=${ overallCachePolicy.maxAge - }, ${overallCachePolicy.scope.toLowerCase()}`, + }, ${overallCachePolicy.scope.toLowerCase()}`, ); } } - return - } - else { + return; + } else { return o.then(p => { if (this.options.calculateHttpHeaders && p.graphqlResponse.http) { const overallCachePolicy = this.computeOverallCachePolicy(); @@ -163,12 +166,12 @@ export class CacheControlExtension 'Cache-Control', `max-age=${ overallCachePolicy.maxAge - }, ${overallCachePolicy.scope.toLowerCase()}`, + }, ${overallCachePolicy.scope.toLowerCase()}`, ); } } - return - }) + return; + }); } } diff --git a/packages/apollo-engine-reporting/src/extension.ts b/packages/apollo-engine-reporting/src/extension.ts index 3618d6d9759..21398c3f865 100644 --- a/packages/apollo-engine-reporting/src/extension.ts +++ b/packages/apollo-engine-reporting/src/extension.ts @@ -18,7 +18,7 @@ import { Trace, google } from 'apollo-engine-reporting-protobuf'; import { EngineReportingOptions, GenerateClientInfo } from './agent'; import { defaultSignature } from './signature'; import { GraphQLRequestContext } from 'apollo-server-core/dist/requestPipelineAPI'; -import {isPromise} from "apollo-server-errors"; +import { isPromise } from 'apollo-server-errors'; const clientNameHeaderKey = 'apollographql-client-name'; const clientReferenceIdHeaderKey = 'apollographql-client-reference-id'; @@ -276,9 +276,13 @@ export class EngineReportingExtension }; } - public willSendResponse(o: { graphqlResponse: GraphQLResponse } | Promise<{ graphqlResponse: GraphQLResponse }>) { + public willSendResponse( + o: + | { graphqlResponse: GraphQLResponse } + | Promise<{ graphqlResponse: GraphQLResponse }>, + ) { if (!isPromise(o)) { - const {errors} = o.graphqlResponse; + const { errors } = o.graphqlResponse; if (errors) { errors.forEach((error: GraphQLError) => { // By default, put errors on the root node. @@ -292,23 +296,22 @@ export class EngineReportingExtension // Always send the trace errors, so that the UI acknowledges that there is an error. const errorInfo = this.options.maskErrorDetails - ? {message: ''} + ? { message: '' } : { - message: error.message, - location: (error.locations || []).map( - ({line, column}) => new Trace.Location({line, column}), - ), - json: JSON.stringify(error), - }; + message: error.message, + location: (error.locations || []).map( + ({ line, column }) => new Trace.Location({ line, column }), + ), + json: JSON.stringify(error), + }; node!.error!.push(new Trace.Error(errorInfo)); }); } - return - } - else { + return; + } else { return o.then(p => { - const {errors} = p.graphqlResponse; + const { errors } = p.graphqlResponse; if (errors) { errors.forEach((error: GraphQLError) => { // By default, put errors on the root node. @@ -322,20 +325,20 @@ export class EngineReportingExtension // Always send the trace errors, so that the UI acknowledges that there is an error. const errorInfo = this.options.maskErrorDetails - ? {message: ''} + ? { message: '' } : { - message: error.message, - location: (error.locations || []).map( - ({line, column}) => new Trace.Location({line, column}), - ), - json: JSON.stringify(error), - }; + message: error.message, + location: (error.locations || []).map( + ({ line, column }) => new Trace.Location({ line, column }), + ), + json: JSON.stringify(error), + }; node!.error!.push(new Trace.Error(errorInfo)); }); } - return - }) + return; + }); } } diff --git a/packages/apollo-server-core/src/formatters.ts b/packages/apollo-server-core/src/formatters.ts index e2013aa74c1..68a131d772c 100644 --- a/packages/apollo-server-core/src/formatters.ts +++ b/packages/apollo-server-core/src/formatters.ts @@ -1,6 +1,5 @@ import { GraphQLExtension, GraphQLResponse } from 'graphql-extensions'; -import {formatApolloErrors} from 'apollo-server-errors'; - +import { formatApolloErrors } from 'apollo-server-errors'; function isPromise(x: any): x is Promise { return x && typeof x.then === 'function'; @@ -16,60 +15,63 @@ export class FormatErrorExtension extends GraphQLExtension { this.debug = debug; } - public willSendResponse(o: Promise<{ - graphqlResponse: GraphQLResponse; - context: TContext; - }> | { - graphqlResponse: GraphQLResponse; - context: TContext; - }): Promise<{ graphqlResponse: GraphQLResponse; context: TContext } | void> | - { graphqlResponse: GraphQLResponse; context: TContext } | void { - - + public willSendResponse( + o: + | Promise<{ + graphqlResponse: GraphQLResponse; + context: TContext; + }> + | { + graphqlResponse: GraphQLResponse; + context: TContext; + }, + ): + | Promise<{ graphqlResponse: GraphQLResponse; context: TContext } | void> + | { graphqlResponse: GraphQLResponse; context: TContext } + | void { if (isPromise(o)) { - return this.asyncWillSendResponse(o) - } - else if(o.graphqlResponse.errors){ - let formattedErrors = formatApolloErrors(o.graphqlResponse.errors, { + return this.asyncWillSendResponse(o); + } else if (o.graphqlResponse.errors) { + let formattedErrors = formatApolloErrors(o.graphqlResponse.errors, { formatter: this.formatError, debug: this.debug, }); - if(isPromise(formattedErrors)){ - return this.asyncWillSendResponse(o) - } - else if(o) { - return ({ + if (isPromise(formattedErrors)) { + return this.asyncWillSendResponse(o); + } else if (o) { + return { ...o, graphqlResponse: { ...o.graphqlResponse, - errors: formattedErrors + errors: formattedErrors, }, - }) + }; } } - return o + return o; } - - public async asyncWillSendResponse(o: Promise<{ - graphqlResponse: GraphQLResponse; - context: TContext; - }> | { graphqlResponse: GraphQLResponse; context: TContext }): Promise<{ graphqlResponse: GraphQLResponse; context: TContext } | void> - { - let p = await o + public async asyncWillSendResponse( + o: + | Promise<{ + graphqlResponse: GraphQLResponse; + context: TContext; + }> + | { graphqlResponse: GraphQLResponse; context: TContext }, + ): Promise<{ graphqlResponse: GraphQLResponse; context: TContext } | void> { + let p = await o; if (p.graphqlResponse.errors) { return { - ...p, - graphqlResponse: { - ...p.graphqlResponse, - errors: await formatApolloErrors(p.graphqlResponse.errors, { + ...p, + graphqlResponse: { + ...p.graphqlResponse, + errors: await formatApolloErrors(p.graphqlResponse.errors, { formatter: this.formatError, debug: this.debug, }), }, }; } - return o + return o; } } - diff --git a/packages/apollo-server-errors/src/index.ts b/packages/apollo-server-errors/src/index.ts index 299be704efc..5c7caac15ab 100644 --- a/packages/apollo-server-errors/src/index.ts +++ b/packages/apollo-server-errors/src/index.ts @@ -251,24 +251,25 @@ export function formatApolloErrors( if (!formatter) { return enrichedErrors; } - if(isPromise(formatter)) { - return Promise.all(enrichedErrors.map(async error => { - try { - return await formatter(error); - } catch (err) { - if (debug) { - return enrichError(err, debug); - } else { - // obscure error - const newError = fromGraphQLError( - new GraphQLError('Internal server error'), - ); - return enrichError(newError, debug); + if (isPromise(formatter)) { + return Promise.all( + enrichedErrors.map(async error => { + try { + return await formatter(error); + } catch (err) { + if (debug) { + return enrichError(err, debug); + } else { + // obscure error + const newError = fromGraphQLError( + new GraphQLError('Internal server error'), + ); + return enrichError(newError, debug); + } } - } - })); - } - else { + }), + ); + } else { return enrichedErrors.map(error => { try { return formatter(error); @@ -283,6 +284,6 @@ export function formatApolloErrors( return enrichError(newError, debug); } } - }) + }); } } diff --git a/packages/apollo-server-integration-testsuite/src/ApolloServer.ts b/packages/apollo-server-integration-testsuite/src/ApolloServer.ts index b189cb20ff7..3300d26f694 100644 --- a/packages/apollo-server-integration-testsuite/src/ApolloServer.ts +++ b/packages/apollo-server-integration-testsuite/src/ApolloServer.ts @@ -641,7 +641,7 @@ export function testApolloServer( throw new Error('nope'); }); - const validationRule = jest.fn( () => { + const validationRule = jest.fn(() => { // formatError should be called after validation expect(formatError).not.toBeCalled(); // extension should be called after validation @@ -653,7 +653,7 @@ export function testApolloServer( const formatError = jest.fn(async error => { try { - await new Promise(resolve => setTimeout(resolve, 500)) + await new Promise(resolve => setTimeout(resolve, 500)); expect(error).toBeInstanceOf(Error); // extension should be called before formatError expect(willSendResponseInExtension).toHaveBeenCalledTimes(1); @@ -699,9 +699,9 @@ export function testApolloServer( const { url: uri } = await createApolloServer({ typeDefs: gql` - type Query { - error: String - } + type Query { + error: String + } `, resolvers: { Query: { @@ -715,7 +715,7 @@ export function testApolloServer( engine: { endpointUrl: `http://localhost:${ (engineServer.address() as net.AddressInfo).port - }`, + }`, apiKey: 'service:my-app:secret', maxUncompressedReportSize: 1, generateClientInfo: () => ({ diff --git a/packages/graphql-extensions/src/index.ts b/packages/graphql-extensions/src/index.ts index 0a9486377d7..90eea9d385d 100644 --- a/packages/graphql-extensions/src/index.ts +++ b/packages/graphql-extensions/src/index.ts @@ -49,14 +49,20 @@ export class GraphQLExtension { executionArgs: ExecutionArgs; }): EndHandler | void; - public willSendResponse?(o: Promise<{ - graphqlResponse: GraphQLResponse; - context: TContext; - }> | { - graphqlResponse: GraphQLResponse; - context: TContext; - }): Promise<{ graphqlResponse: GraphQLResponse; context: TContext } | void> | - { graphqlResponse: GraphQLResponse; context: TContext } | void + public willSendResponse?( + o: + | Promise<{ + graphqlResponse: GraphQLResponse; + context: TContext; + }> + | { + graphqlResponse: GraphQLResponse; + context: TContext; + }, + ): + | Promise<{ graphqlResponse: GraphQLResponse; context: TContext } | void> + | { graphqlResponse: GraphQLResponse; context: TContext } + | void; public willResolveField?( source: any, @@ -112,36 +118,40 @@ export class GraphQLExtensionStack { ); } - public willSendResponse(o: Promise<{ - graphqlResponse: GraphQLResponse; - context: TContext; - }> | { - graphqlResponse: GraphQLResponse; - context: TContext; - }): Promise<{ graphqlResponse: GraphQLResponse; context: TContext }> | { - graphqlResponse: GraphQLResponse; - context: TContext; - } { - + public willSendResponse( + o: + | Promise<{ + graphqlResponse: GraphQLResponse; + context: TContext; + }> + | { + graphqlResponse: GraphQLResponse; + context: TContext; + }, + ): + | Promise<{ graphqlResponse: GraphQLResponse; context: TContext }> + | { + graphqlResponse: GraphQLResponse; + context: TContext; + } { // Reverse the array, since this is functions as an end handler - return [...this.extensions].reverse().reduce((promiseChain, currentTask) => { - if(currentTask.willSendResponse) { - let possiblyAPromise = currentTask.willSendResponse(promiseChain) - if(possiblyAPromise && isPromise(possiblyAPromise)){ - return possiblyAPromise.then(res => res || promiseChain) - } - else if(possiblyAPromise) { - return possiblyAPromise - } - else { - return promiseChain + return [...this.extensions] + .reverse() + .reduce((promiseChain, currentTask) => { + if (currentTask.willSendResponse) { + let possiblyAPromise = currentTask.willSendResponse(promiseChain); + if (possiblyAPromise && isPromise(possiblyAPromise)) { + return possiblyAPromise.then(res => res || promiseChain); + } else if (possiblyAPromise) { + return possiblyAPromise; + } else { + return promiseChain; + } + } else { + return promiseChain; } - } - else { - return promiseChain - } - }, o); + }, o); } public willResolveField( From 6b0e29bf00bbac956a6d67aad5b8ed0dc7216a14 Mon Sep 17 00:00:00 2001 From: Antoine Sauvage Date: Mon, 10 Dec 2018 18:06:47 +0100 Subject: [PATCH 4/4] independant isPromise --- packages/apollo-cache-control/src/index.ts | 5 ++++- packages/apollo-engine-reporting/src/extension.ts | 5 ++++- packages/apollo-server-errors/src/index.ts | 2 +- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/apollo-cache-control/src/index.ts b/packages/apollo-cache-control/src/index.ts index 958c9728d3b..d6f2aaa961b 100644 --- a/packages/apollo-cache-control/src/index.ts +++ b/packages/apollo-cache-control/src/index.ts @@ -9,7 +9,10 @@ import { } from 'graphql'; import { GraphQLExtension, GraphQLResponse } from 'graphql-extensions'; -import { isPromise } from 'apollo-server-errors'; + +function isPromise(x: any): x is Promise { + return x && typeof x.then === 'function'; +} export interface CacheControlFormat { version: 1; diff --git a/packages/apollo-engine-reporting/src/extension.ts b/packages/apollo-engine-reporting/src/extension.ts index 21398c3f865..b0c39c1cce8 100644 --- a/packages/apollo-engine-reporting/src/extension.ts +++ b/packages/apollo-engine-reporting/src/extension.ts @@ -18,7 +18,10 @@ import { Trace, google } from 'apollo-engine-reporting-protobuf'; import { EngineReportingOptions, GenerateClientInfo } from './agent'; import { defaultSignature } from './signature'; import { GraphQLRequestContext } from 'apollo-server-core/dist/requestPipelineAPI'; -import { isPromise } from 'apollo-server-errors'; + +function isPromise(x: any): x is Promise { + return x && typeof x.then === 'function'; +} const clientNameHeaderKey = 'apollographql-client-name'; const clientReferenceIdHeaderKey = 'apollographql-client-reference-id'; diff --git a/packages/apollo-server-errors/src/index.ts b/packages/apollo-server-errors/src/index.ts index 5c7caac15ab..1dc3846635c 100644 --- a/packages/apollo-server-errors/src/index.ts +++ b/packages/apollo-server-errors/src/index.ts @@ -1,6 +1,6 @@ import { GraphQLError } from 'graphql'; -export function isPromise(x: any): x is Promise { +function isPromise(x: any): x is Promise { return x && typeof x.then === 'function'; }