Skip to content

Commit

Permalink
formatError: put all extensions inside 'extensions' property (#1284)
Browse files Browse the repository at this point in the history
* formatError: put all extensions inside 'extensions' property

* Address @leebyron review comments
  • Loading branch information
IvanGoncharov authored and leebyron committed Apr 6, 2018
1 parent 92cb4a0 commit 0bb47b2
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 13 deletions.
13 changes: 11 additions & 2 deletions src/error/GraphQLError.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ declare class GraphQLError extends Error {
/**
* Extension fields to add to the formatted error.
*/
+extensions: ?{ [key: string]: mixed };
+extensions: { [key: string]: mixed } | void;
}

export function GraphQLError( // eslint-disable-line no-redeclare
Expand Down Expand Up @@ -135,6 +135,9 @@ export function GraphQLError( // eslint-disable-line no-redeclare
}, []);
}

const _extensions =
extensions || (originalError && (originalError: any).extensions);

Object.defineProperties(this, {
message: {
value: message,
Expand Down Expand Up @@ -175,7 +178,13 @@ export function GraphQLError( // eslint-disable-line no-redeclare
value: originalError,
},
extensions: {
value: extensions || (originalError && (originalError: any).extensions),
// Coercing falsey values to undefined ensures they will not be included
// in JSON.stringify() when not provided.
value: _extensions || undefined,
// By being enumerable, JSON.stringify will include `path` in the
// resulting output. This ensures that the simplest possible GraphQL
// service adheres to the spec.
enumerable: Boolean(_extensions),
},
});

Expand Down
2 changes: 1 addition & 1 deletion src/error/__tests__/GraphQLError-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ describe('GraphQLError', () => {
message: 'msg',
locations: undefined,
path: undefined,
foo: 'bar',
extensions: { foo: 'bar' },
});
});
});
21 changes: 11 additions & 10 deletions src/error/formatError.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,19 @@ import type { SourceLocation } from '../language/location';
*/
export function formatError(error: GraphQLError): GraphQLFormattedError {
invariant(error, 'Received null or undefined error.');
return {
...error.extensions,
message: error.message || 'An unknown error occurred.',
locations: error.locations,
path: error.path,
};
const message = error.message || 'An unknown error occurred.';
const locations = error.locations;
const path = error.path;
const extensions = error.extensions;

return extensions
? { message, locations, path, extensions }
: { message, locations, path };
}

export type GraphQLFormattedError = {
export type GraphQLFormattedError = {|
+message: string,
+locations: $ReadOnlyArray<SourceLocation> | void,
+path: $ReadOnlyArray<string | number> | void,
// Extensions
+[key: string]: mixed,
};
+extensions?: { [key: string]: mixed },
|};
16 changes: 16 additions & 0 deletions src/execution/__tests__/executor-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,7 @@ describe('Execute: Handles basic execution tasks', () => {
asyncError
asyncRawError
asyncReturnError
asyncReturnErrorWithExtensions
}`;

const data = {
Expand Down Expand Up @@ -435,6 +436,12 @@ describe('Execute: Handles basic execution tasks', () => {
asyncReturnError() {
return Promise.resolve(new Error('Error getting asyncReturnError'));
},
asyncReturnErrorWithExtensions() {
const error = new Error('Error getting asyncReturnErrorWithExtensions');
error.extensions = { foo: 'bar' };

return Promise.resolve(error);
},
};

const ast = parse(doc);
Expand All @@ -449,11 +456,13 @@ describe('Execute: Handles basic execution tasks', () => {
syncReturnErrorList: { type: GraphQLList(GraphQLString) },
async: { type: GraphQLString },
asyncReject: { type: GraphQLString },
asyncRejectWithExtensions: { type: GraphQLString },
asyncRawReject: { type: GraphQLString },
asyncEmptyReject: { type: GraphQLString },
asyncError: { type: GraphQLString },
asyncRawError: { type: GraphQLString },
asyncReturnError: { type: GraphQLString },
asyncReturnErrorWithExtensions: { type: GraphQLString },
},
}),
});
Expand All @@ -474,6 +483,7 @@ describe('Execute: Handles basic execution tasks', () => {
asyncError: null,
asyncRawError: null,
asyncReturnError: null,
asyncReturnErrorWithExtensions: null,
},
errors: [
{
Expand Down Expand Up @@ -531,6 +541,12 @@ describe('Execute: Handles basic execution tasks', () => {
locations: [{ line: 13, column: 7 }],
path: ['asyncReturnError'],
},
{
message: 'Error getting asyncReturnErrorWithExtensions',
locations: [{ line: 14, column: 7 }],
path: ['asyncReturnErrorWithExtensions'],
extensions: { foo: 'bar' },
},
],
});
});
Expand Down

0 comments on commit 0bb47b2

Please sign in to comment.