Skip to content

Commit 0bb47b2

Browse files
IvanGoncharovleebyron
authored andcommitted
formatError: put all extensions inside 'extensions' property (#1284)
* formatError: put all extensions inside 'extensions' property * Address @leebyron review comments
1 parent 92cb4a0 commit 0bb47b2

File tree

4 files changed

+39
-13
lines changed

4 files changed

+39
-13
lines changed

src/error/GraphQLError.js

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ declare class GraphQLError extends Error {
8686
/**
8787
* Extension fields to add to the formatted error.
8888
*/
89-
+extensions: ?{ [key: string]: mixed };
89+
+extensions: { [key: string]: mixed } | void;
9090
}
9191

9292
export function GraphQLError( // eslint-disable-line no-redeclare
@@ -135,6 +135,9 @@ export function GraphQLError( // eslint-disable-line no-redeclare
135135
}, []);
136136
}
137137

138+
const _extensions =
139+
extensions || (originalError && (originalError: any).extensions);
140+
138141
Object.defineProperties(this, {
139142
message: {
140143
value: message,
@@ -175,7 +178,13 @@ export function GraphQLError( // eslint-disable-line no-redeclare
175178
value: originalError,
176179
},
177180
extensions: {
178-
value: extensions || (originalError && (originalError: any).extensions),
181+
// Coercing falsey values to undefined ensures they will not be included
182+
// in JSON.stringify() when not provided.
183+
value: _extensions || undefined,
184+
// By being enumerable, JSON.stringify will include `path` in the
185+
// resulting output. This ensures that the simplest possible GraphQL
186+
// service adheres to the spec.
187+
enumerable: Boolean(_extensions),
179188
},
180189
});
181190

src/error/__tests__/GraphQLError-test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ describe('GraphQLError', () => {
148148
message: 'msg',
149149
locations: undefined,
150150
path: undefined,
151-
foo: 'bar',
151+
extensions: { foo: 'bar' },
152152
});
153153
});
154154
});

src/error/formatError.js

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,19 @@ import type { SourceLocation } from '../language/location';
1717
*/
1818
export function formatError(error: GraphQLError): GraphQLFormattedError {
1919
invariant(error, 'Received null or undefined error.');
20-
return {
21-
...error.extensions,
22-
message: error.message || 'An unknown error occurred.',
23-
locations: error.locations,
24-
path: error.path,
25-
};
20+
const message = error.message || 'An unknown error occurred.';
21+
const locations = error.locations;
22+
const path = error.path;
23+
const extensions = error.extensions;
24+
25+
return extensions
26+
? { message, locations, path, extensions }
27+
: { message, locations, path };
2628
}
2729

28-
export type GraphQLFormattedError = {
30+
export type GraphQLFormattedError = {|
2931
+message: string,
3032
+locations: $ReadOnlyArray<SourceLocation> | void,
3133
+path: $ReadOnlyArray<string | number> | void,
32-
// Extensions
33-
+[key: string]: mixed,
34-
};
34+
+extensions?: { [key: string]: mixed },
35+
|};

src/execution/__tests__/executor-test.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,7 @@ describe('Execute: Handles basic execution tasks', () => {
382382
asyncError
383383
asyncRawError
384384
asyncReturnError
385+
asyncReturnErrorWithExtensions
385386
}`;
386387

387388
const data = {
@@ -435,6 +436,12 @@ describe('Execute: Handles basic execution tasks', () => {
435436
asyncReturnError() {
436437
return Promise.resolve(new Error('Error getting asyncReturnError'));
437438
},
439+
asyncReturnErrorWithExtensions() {
440+
const error = new Error('Error getting asyncReturnErrorWithExtensions');
441+
error.extensions = { foo: 'bar' };
442+
443+
return Promise.resolve(error);
444+
},
438445
};
439446

440447
const ast = parse(doc);
@@ -449,11 +456,13 @@ describe('Execute: Handles basic execution tasks', () => {
449456
syncReturnErrorList: { type: GraphQLList(GraphQLString) },
450457
async: { type: GraphQLString },
451458
asyncReject: { type: GraphQLString },
459+
asyncRejectWithExtensions: { type: GraphQLString },
452460
asyncRawReject: { type: GraphQLString },
453461
asyncEmptyReject: { type: GraphQLString },
454462
asyncError: { type: GraphQLString },
455463
asyncRawError: { type: GraphQLString },
456464
asyncReturnError: { type: GraphQLString },
465+
asyncReturnErrorWithExtensions: { type: GraphQLString },
457466
},
458467
}),
459468
});
@@ -474,6 +483,7 @@ describe('Execute: Handles basic execution tasks', () => {
474483
asyncError: null,
475484
asyncRawError: null,
476485
asyncReturnError: null,
486+
asyncReturnErrorWithExtensions: null,
477487
},
478488
errors: [
479489
{
@@ -531,6 +541,12 @@ describe('Execute: Handles basic execution tasks', () => {
531541
locations: [{ line: 13, column: 7 }],
532542
path: ['asyncReturnError'],
533543
},
544+
{
545+
message: 'Error getting asyncReturnErrorWithExtensions',
546+
locations: [{ line: 14, column: 7 }],
547+
path: ['asyncReturnErrorWithExtensions'],
548+
extensions: { foo: 'bar' },
549+
},
534550
],
535551
});
536552
});

0 commit comments

Comments
 (0)