Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release-2.5.0] Fix missing operationName in apollo-engine-reporting. #2557

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import {
VERSION,
} from 'apollo-link-persisted-queries';

import { createApolloFetch, GraphQLRequest } from 'apollo-fetch';
import { createApolloFetch, ApolloFetch, GraphQLRequest } from 'apollo-fetch';
import {
AuthenticationError,
UserInputError,
Expand Down Expand Up @@ -586,6 +586,11 @@ export function testApolloServer<AS extends ApolloServerBase>(
(engineServer.stop() || Promise.resolve()).then(done);
});

// TODO(jesse) This test block can be merged with another that will
// appear in a separate describe/it block after a branch merges
// that hasn't quite become a PR yet: abernix/finish-pr-1639. It's
// intentionally separate right now because of the guaranteed merge
// conflict.
it('validation > engine > extensions > formatError', async () => {
const throwError = jest.fn(() => {
throw new Error('nope');
Expand Down Expand Up @@ -686,6 +691,72 @@ export function testApolloServer<AS extends ApolloServerBase>(
expect(trace.root!.child![0].error![0].message).toMatch(/nope/);
expect(trace.root!.child![0].error![0].message).not.toMatch(/masked/);
});

describe('validation > engine > traces', () => {
let apolloFetch: ApolloFetch;

const setupApolloServerAndFetchPair = async (
engineOptions: Partial<EngineReportingOptions<any>> = {},
) => {
const { url: uri } = await createApolloServer({
typeDefs: gql`
type Query {
justAField: String
}
`,
resolvers: {
Query: {
justAField: () => 'a string',
},
},
engine: {
...engineServer.engineOptions(),
apiKey: 'service:my-app:secret',
maxUncompressedReportSize: 1,
...engineOptions,
},
debug: true,
});

apolloFetch = createApolloFetch({ uri });
};

it('sets the trace key to operationName when it is defined', async () => {
await setupApolloServerAndFetchPair();

const result = await apolloFetch({
query: `query AnOperationName {justAField}`,
});
expect(result.data).toEqual({
justAField: 'a string',
});
expect(result.errors).not.toBeDefined();

const reports = await engineServer.promiseOfReports;
expect(reports.length).toBe(1);

expect(Object.keys(reports[0].tracesPerQuery)[0]).toMatch(
/^# AnOperationName\n/,
);
});

it('sets the trace key to "-" when operationName is undefined', async () => {
await setupApolloServerAndFetchPair();

const result = await apolloFetch({
query: `{justAField}`,
});
expect(result.data).toEqual({
justAField: 'a string',
});
expect(result.errors).not.toBeDefined();

const reports = await engineServer.promiseOfReports;
expect(reports.length).toBe(1);

expect(Object.keys(reports[0].tracesPerQuery)[0]).toMatch(/^# -\n/);
});
});
});

it('errors thrown in extensions call formatError and are wrapped', async () => {
Expand Down