From e101cffd235a667324f89dcfdeec3cf77d9514dc Mon Sep 17 00:00:00 2001 From: Jesse Rosenberger Date: Tue, 9 Apr 2019 23:47:19 +0300 Subject: [PATCH] fix: Set `operationName` via `executionDidStart` and `willResolveField`. The full-query caching feature implemented in https://github.com/apollographql/apollo-server/pull/2437 originally took the additional steps of also changing the `apollo-engine-reporting` module to utilize the new request pipeline available in Apollo Server as of https://github.com/apollographql/apollo-server/pull/1795. While that would have been nice, there was still some uncertainty about some of the life-cycle hooks that would be necesssary to make that happen and it wasn't worth blocking the implementation of full-query caching on those stalled decisions. Therefore, the changes to utilize new functionality in the request pipeline, including what would have been a simplification of the way that `apollo-engine-reporting` would have obtained the `operationName` (something that is available via the API of the request-pipeline hooks), were backed out and will land separately in a future PR. The portion regarding `operationName` was inadvertently not backed out and instead was left leveraging a life-cycle hook which was not available to the `graphql-extensions` API: `didResolveOperation`. This means that the code was not setting `operationName` in the way it needed to and therefore `operationName` was always being left undefined (as is sometimes permitted!) with this new `apollo-engine-reporting`. This commit puts the functionality back to that which is required for the `graphql-extensions` implementation, and the commit before this (8a43341a) acts as a regression test (it should pass, as of this commit, and fail before it). --- .../apollo-engine-reporting/src/extension.ts | 28 ++++++++++++++----- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/packages/apollo-engine-reporting/src/extension.ts b/packages/apollo-engine-reporting/src/extension.ts index 8525a2234ec..cfa4bccb051 100644 --- a/packages/apollo-engine-reporting/src/extension.ts +++ b/packages/apollo-engine-reporting/src/extension.ts @@ -5,6 +5,7 @@ import { responsePathAsArray, ResponsePath, DocumentNode, + ExecutionArgs, GraphQLError, } from 'graphql'; import { @@ -237,13 +238,21 @@ export class EngineReportingExtension }; } - public didResolveOperation(o: { - requestContext: GraphQLRequestContext; - }) { - const { requestContext } = o; - - this.operationName = requestContext.operationName; - this.documentAST = requestContext.document; + public executionDidStart(o: { executionArgs: ExecutionArgs }) { + // If the operationName is explicitly provided, save it. If there's just one + // named operation, the client doesn't have to provide it, but we still want + // to know the operation name so that the server can identify the query by + // it without having to parse a signature. + // + // Fortunately, in the non-error case, we can just pull this out of + // the first call to willResolveField's `info` argument. In an + // error case (eg, the operationName isn't found, or there are more + // than one operation and no specified operationName) it's OK to continue + // to file this trace under the empty operationName. + if (o.executionArgs.operationName) { + this.operationName = o.executionArgs.operationName; + } + this.documentAST = o.executionArgs.document; } public willResolveField( @@ -252,6 +261,11 @@ export class EngineReportingExtension _context: TContext, info: GraphQLResolveInfo, ): ((error: Error | null, result: any) => void) | void { + if (this.operationName === undefined) { + this.operationName = + (info.operation.name && info.operation.name.value) || ''; + } + const path = info.path; const node = this.newNode(path); node.type = info.returnType.toString();