diff --git a/packages/apollo-server-core/src/__tests__/errors.test.ts b/packages/apollo-server-core/src/__tests__/errors.test.ts index 040eb2dd086..fbcaa8ea9a8 100644 --- a/packages/apollo-server-core/src/__tests__/errors.test.ts +++ b/packages/apollo-server-core/src/__tests__/errors.test.ts @@ -20,11 +20,12 @@ describe('Errors', () => { }); it('allows code setting and additional properties', () => { const code = 'CODE'; - const key = 'key'; + const key = 'value'; const error = new ApolloError(message, code, { key }); expect(error.message).toEqual(message); - expect(error.key).toEqual(key); + expect(error.key).toBeUndefined(); expect(error.extensions.code).toEqual(code); + expect(error.extensions.key).toEqual(key); }); }); @@ -37,7 +38,7 @@ describe('Errors', () => { | ((options?: Record) => Record); const message = 'message'; const code = 'CODE'; - const key = 'key'; + const key = 'value'; const createFormattedError: CreateFormatError = ( options?: Record, @@ -66,7 +67,8 @@ describe('Errors', () => { it('exposes a stacktrace in debug mode', () => { const error = createFormattedError({ debug: true }); expect(error.message).toEqual(message); - expect(error.extensions.exception.key).toEqual(key); + expect(error.extensions.key).toEqual(key); + expect(error.extensions.exception.key).toBeUndefined(); expect(error.extensions.code).toEqual(code); // stacktrace should exist under exception expect(error.extensions.exception.stacktrace).toBeDefined(); @@ -93,10 +95,9 @@ describe('Errors', () => { it('exposes fields on error under exception field and provides code', () => { const error = createFormattedError(); expect(error.message).toEqual(message); - expect(error.extensions.exception.key).toEqual(key); + expect(error.extensions.key).toEqual(key); + expect(error.extensions.exception).toBeUndefined(); expect(error.extensions.code).toEqual(code); - // stacktrace should exist under exception - expect(error.extensions.exception.stacktrace).toBeUndefined(); }); it('calls formatter after exposing the code and stacktrace', () => { const error = new ApolloError(message, code, { key }); @@ -106,7 +107,8 @@ describe('Errors', () => { debug: true, }); expect(error.message).toEqual(message); - expect(error.key).toEqual(key); + expect(error.extensions.key).toEqual(key); + expect(error.key).toBeUndefined(); expect(error.extensions.code).toEqual(code); expect(error instanceof ApolloError).toBe(true); expect(formatter).toHaveBeenCalledTimes(1); @@ -184,8 +186,9 @@ describe('Errors', () => { ), ])[0]; - expect(formattedError.extensions.exception.field1).toEqual('property1'); - expect(formattedError.extensions.exception.field2).toEqual('property2'); + expect(formattedError.extensions.field1).toEqual('property1'); + expect(formattedError.extensions.field2).toEqual('property2'); + expect(formattedError.extensions.exception).toBeUndefined(); }); }); }); diff --git a/packages/apollo-server-errors/src/__tests__/ApolloError.test.ts b/packages/apollo-server-errors/src/__tests__/ApolloError.test.ts index 5fdb2587be6..b1ba85ebc89 100644 --- a/packages/apollo-server-errors/src/__tests__/ApolloError.test.ts +++ b/packages/apollo-server-errors/src/__tests__/ApolloError.test.ts @@ -37,13 +37,14 @@ describe('ApolloError', () => { ); }); - it('(back-compat) sets extensions correctly for users who use an extensions key in the third constructor argument', () => { - const error = new ApolloError('My original message', 'A_CODE', { - extensions: { - arbitrary: 'user_data', - }, - }); - - expect(error.extensions.arbitrary).toEqual('user_data'); + it('throws for users who use an extensions key in the third constructor argument', () => { + expect( + () => + new ApolloError('My original message', 'A_CODE', { + extensions: { + arbitrary: 'user_data', + }, + }), + ).toThrow(/Pass extensions directly/); }); }); diff --git a/packages/apollo-server-errors/src/index.ts b/packages/apollo-server-errors/src/index.ts index dd480cde08d..4cfc0da45ff 100644 --- a/packages/apollo-server-errors/src/index.ts +++ b/packages/apollo-server-errors/src/index.ts @@ -1,4 +1,10 @@ -import { ASTNode, GraphQLError, GraphQLFormattedError, Source, SourceLocation } from 'graphql'; +import { + ASTNode, + GraphQLError, + GraphQLFormattedError, + Source, + SourceLocation, +} from 'graphql'; export class ApolloError extends Error implements GraphQLError { public extensions: Record; @@ -19,39 +25,20 @@ export class ApolloError extends Error implements GraphQLError { ) { super(message); - // This variable was previously named `properties`, which allowed users to set - // arbitrary properties on the ApolloError object. This use case is still supported, - // but deprecated in favor of using the ApolloError.extensions object instead. - // This change intends to comply with the GraphQL spec on errors. See: - // https://github.com/graphql/graphql-spec/blob/master/spec/Section%207%20--%20Response.md#response-format - // - // Going forward, users should use the ApolloError.extensions object for storing - // and reading arbitrary data on an error, as arbitrary properties on the ApolloError - // itself won't be supported in the future. - // - // XXX Filter 'message' and 'extensions' specifically so they don't overwrite the class property. - // We _could_ filter all of the class properties, but have chosen to only do - // so if it's an issue for other users. Please feel free to open an issue if you - // find yourself here with this exact problem. - if (extensions) { - Object.keys(extensions) - .filter(keyName => keyName !== 'message' && keyName !== 'extensions') - .forEach(key => { - this[key] = extensions[key]; - }); - } - // if no name provided, use the default. defineProperty ensures that it stays non-enumerable if (!this.name) { Object.defineProperty(this, 'name', { value: 'ApolloError' }); } - // Before the mentioned change to extensions, users could previously set the extensions - // object by providing it as a key on the third argument to the constructor. - // This step provides backwards compatibility for those hypothetical users. - const userProvidedExtensions = (extensions && extensions.extensions) || null; + if (extensions?.extensions) { + throw Error( + 'Pass extensions directly as the third argument of the ApolloError constructor: `new ' + + 'ApolloError(message, code, {myExt: value})`, not `new ApolloError(message, code, ' + + '{extensions: {myExt: value}})`', + ); + } - this.extensions = { ...extensions, ...userProvidedExtensions, code }; + this.extensions = { ...extensions, code }; } } @@ -164,8 +151,10 @@ export function fromGraphQLError(error: GraphQLError, options?: ErrorOptions) { // copy the original error, while keeping all values non-enumerable, so they // are not printed unless directly referenced Object.defineProperty(copy, 'originalError', { value: {} }); - Object.getOwnPropertyNames(error).forEach(key => { - Object.defineProperty(copy.originalError, key, { value: (error as any)[key] }); + Object.getOwnPropertyNames(error).forEach((key) => { + Object.defineProperty(copy.originalError, key, { + value: (error as any)[key], + }); }); return copy; @@ -224,8 +213,8 @@ export class PersistedQueryNotSupportedError extends ApolloError { } export class UserInputError extends ApolloError { - constructor(message: string, properties?: Record) { - super(message, 'BAD_USER_INPUT', properties); + constructor(message: string, extensions?: Record) { + super(message, 'BAD_USER_INPUT', extensions); Object.defineProperty(this, 'name', { value: 'UserInputError' }); } @@ -239,7 +228,7 @@ export function formatApolloErrors( }, ): Array { if (!options) { - return errors.map(error => enrichError(error)); + return errors.map((error) => enrichError(error)); } const { formatter, debug } = options; @@ -263,7 +252,7 @@ export function formatApolloErrors( // flattenedErrors.push(error); // } - const enrichedErrors = errors.map(error => enrichError(error, debug)); + const enrichedErrors = errors.map((error) => enrichError(error, debug)); const makePrintable = (error: GraphQLFormattedError) => { if (error instanceof Error) { // Error defines its `message` and other fields as non-enumerable, meaning JSON.stringigfy does not print them. @@ -282,7 +271,7 @@ export function formatApolloErrors( return enrichedErrors; } - return enrichedErrors.map(error => { + return enrichedErrors.map((error) => { try { return makePrintable(formatter(error)); } catch (err) { diff --git a/packages/apollo-server-integration-testsuite/src/ApolloServer.ts b/packages/apollo-server-integration-testsuite/src/ApolloServer.ts index 965c344d9ca..1f620592ed0 100644 --- a/packages/apollo-server-integration-testsuite/src/ApolloServer.ts +++ b/packages/apollo-server-integration-testsuite/src/ApolloServer.ts @@ -45,6 +45,7 @@ import { ApolloServerPluginUsageReportingOptions, ApolloServerPluginLandingPageDisabled, ApolloServerPluginLandingPageGraphQLPlayground, + ApolloError, } from 'apollo-server-core'; import { Headers, fetch } from 'apollo-server-env'; import ApolloServerPluginResponseCache from 'apollo-server-plugin-response-cache'; @@ -1649,6 +1650,40 @@ export function testApolloServer( expect(result.errors[0].extensions.code).toEqual('UNAUTHENTICATED'); expect(result.errors[0].extensions.exception).toBeUndefined(); }); + + it('shows ApolloError extensions in extensions (only!)', async () => { + const { url: uri } = await createApolloServer({ + typeDefs: gql` + type Query { + fieldWhichWillError: String + } + `, + resolvers: { + Query: { + fieldWhichWillError: () => { + throw new ApolloError('Some message', 'SOME_CODE', { + ext1: 'myext', + }); + }, + }, + }, + stopOnTerminationSignals: false, + __testing_nodeEnv__: 'development', + }); + + const apolloFetch = createApolloFetch({ uri }); + + const result = await apolloFetch({ query: `{fieldWhichWillError}` }); + expect(result.data).toEqual({ fieldWhichWillError: null }); + + expect(result.errors).toBeDefined(); + expect(result.errors.length).toEqual(1); + expect(result.errors[0].message).toEqual('Some message'); + expect(result.errors[0].extensions.code).toEqual('SOME_CODE'); + expect(result.errors[0].extensions.ext1).toEqual('myext'); + expect(result.errors[0].extensions.exception).toBeDefined(); + expect(result.errors[0].extensions.exception.ext1).toBeUndefined(); + }); }); describe('Persisted Queries', () => {