Skip to content

Commit

Permalink
fix: Set operationName via executionDidStart and willResolveField.
Browse files Browse the repository at this point in the history
The full-query caching feature implemented in
#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
#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
(1ae7def) acts as a regression test (it
should pass, as of this commit, and fail before it).
  • Loading branch information
abernix committed Apr 9, 2019
1 parent 8a43341 commit 145fa73
Showing 1 changed file with 21 additions and 7 deletions.
28 changes: 21 additions & 7 deletions packages/apollo-engine-reporting/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
responsePathAsArray,
ResponsePath,
DocumentNode,
ExecutionArgs,
GraphQLError,
} from 'graphql';
import {
Expand Down Expand Up @@ -237,13 +238,21 @@ export class EngineReportingExtension<TContext = any>
};
}

public didResolveOperation(o: {
requestContext: GraphQLRequestContext<TContext>;
}) {
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(
Expand All @@ -252,6 +261,11 @@ export class EngineReportingExtension<TContext = any>
_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();
Expand Down

0 comments on commit 145fa73

Please sign in to comment.