Skip to content

Commit 30f6e73

Browse files
IvanGoncharovIvanGoncharov
committed
Error: make 'path' and 'locations' enumerable only if present
Also cleanup tests from 'undefined' and 'containSubset'.
1 parent 6f16ba2 commit 30f6e73

40 files changed

+187
-355
lines changed

‎src/error/GraphQLError.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ export function GraphQLError( // eslint-disable-line no-redeclare
151151
// By being enumerable, JSON.stringify will include `locations` in the
152152
// resulting output. This ensures that the simplest possible GraphQL
153153
// service adheres to the spec.
154-
enumerable: true,
154+
enumerable: Boolean(_locations),
155155
},
156156
path: {
157157
// Coercing falsey values to undefined ensures they will not be included
@@ -160,7 +160,7 @@ export function GraphQLError( // eslint-disable-line no-redeclare
160160
// By being enumerable, JSON.stringify will include `path` in the
161161
// resulting output. This ensures that the simplest possible GraphQL
162162
// service adheres to the spec.
163-
enumerable: true,
163+
enumerable: Boolean(path),
164164
},
165165
nodes: {
166166
value: _nodes || undefined,

‎src/execution/__tests__/executor-test.js

Lines changed: 87 additions & 100 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
import { expect } from 'chai';
99
import { describe, it } from 'mocha';
1010
import { execute } from '../execute';
11-
import { formatError } from '../../error';
1211
import { parse } from '../../language';
1312
import {
1413
GraphQLSchema,
@@ -461,78 +460,79 @@ describe('Execute: Handles basic execution tasks', () => {
461460

462461
const result = await execute(schema, ast, data);
463462

464-
expect(result.data).to.deep.equal({
465-
sync: 'sync',
466-
syncError: null,
467-
syncRawError: null,
468-
syncReturnError: null,
469-
syncReturnErrorList: ['sync0', null, 'sync2', null],
470-
async: 'async',
471-
asyncReject: null,
472-
asyncRawReject: null,
473-
asyncEmptyReject: null,
474-
asyncError: null,
475-
asyncRawError: null,
476-
asyncReturnError: null,
477-
});
478-
479-
expect(result.errors && result.errors.map(formatError)).to.deep.equal([
480-
{
481-
message: 'Error getting syncError',
482-
locations: [{ line: 3, column: 7 }],
483-
path: ['syncError'],
484-
},
485-
{
486-
message: 'Error getting syncRawError',
487-
locations: [{ line: 4, column: 7 }],
488-
path: ['syncRawError'],
489-
},
490-
{
491-
message: 'Error getting syncReturnError',
492-
locations: [{ line: 5, column: 7 }],
493-
path: ['syncReturnError'],
494-
},
495-
{
496-
message: 'Error getting syncReturnErrorList1',
497-
locations: [{ line: 6, column: 7 }],
498-
path: ['syncReturnErrorList', 1],
499-
},
500-
{
501-
message: 'Error getting syncReturnErrorList3',
502-
locations: [{ line: 6, column: 7 }],
503-
path: ['syncReturnErrorList', 3],
504-
},
505-
{
506-
message: 'Error getting asyncReject',
507-
locations: [{ line: 8, column: 7 }],
508-
path: ['asyncReject'],
509-
},
510-
{
511-
message: 'Error getting asyncRawReject',
512-
locations: [{ line: 9, column: 7 }],
513-
path: ['asyncRawReject'],
514-
},
515-
{
516-
message: 'An unknown error occurred.',
517-
locations: [{ line: 10, column: 7 }],
518-
path: ['asyncEmptyReject'],
519-
},
520-
{
521-
message: 'Error getting asyncError',
522-
locations: [{ line: 11, column: 7 }],
523-
path: ['asyncError'],
524-
},
525-
{
526-
message: 'Error getting asyncRawError',
527-
locations: [{ line: 12, column: 7 }],
528-
path: ['asyncRawError'],
529-
},
530-
{
531-
message: 'Error getting asyncReturnError',
532-
locations: [{ line: 13, column: 7 }],
533-
path: ['asyncReturnError'],
463+
expect(result).to.deep.equal({
464+
data: {
465+
sync: 'sync',
466+
syncError: null,
467+
syncRawError: null,
468+
syncReturnError: null,
469+
syncReturnErrorList: ['sync0', null, 'sync2', null],
470+
async: 'async',
471+
asyncReject: null,
472+
asyncRawReject: null,
473+
asyncEmptyReject: null,
474+
asyncError: null,
475+
asyncRawError: null,
476+
asyncReturnError: null,
534477
},
535-
]);
478+
errors: [
479+
{
480+
message: 'Error getting syncError',
481+
locations: [{ line: 3, column: 7 }],
482+
path: ['syncError'],
483+
},
484+
{
485+
message: 'Error getting syncRawError',
486+
locations: [{ line: 4, column: 7 }],
487+
path: ['syncRawError'],
488+
},
489+
{
490+
message: 'Error getting syncReturnError',
491+
locations: [{ line: 5, column: 7 }],
492+
path: ['syncReturnError'],
493+
},
494+
{
495+
message: 'Error getting syncReturnErrorList1',
496+
locations: [{ line: 6, column: 7 }],
497+
path: ['syncReturnErrorList', 1],
498+
},
499+
{
500+
message: 'Error getting syncReturnErrorList3',
501+
locations: [{ line: 6, column: 7 }],
502+
path: ['syncReturnErrorList', 3],
503+
},
504+
{
505+
message: 'Error getting asyncReject',
506+
locations: [{ line: 8, column: 7 }],
507+
path: ['asyncReject'],
508+
},
509+
{
510+
message: 'Error getting asyncRawReject',
511+
locations: [{ line: 9, column: 7 }],
512+
path: ['asyncRawReject'],
513+
},
514+
{
515+
message: '',
516+
locations: [{ line: 10, column: 7 }],
517+
path: ['asyncEmptyReject'],
518+
},
519+
{
520+
message: 'Error getting asyncError',
521+
locations: [{ line: 11, column: 7 }],
522+
path: ['asyncError'],
523+
},
524+
{
525+
message: 'Error getting asyncRawError',
526+
locations: [{ line: 12, column: 7 }],
527+
path: ['asyncRawError'],
528+
},
529+
{
530+
message: 'Error getting asyncReturnError',
531+
locations: [{ line: 13, column: 7 }],
532+
path: ['asyncReturnError'],
533+
},
534+
],
535+
});
536536
});
537537

538538
it('nulls error subtree for promise rejection #1071', async () => {
@@ -720,13 +720,7 @@ describe('Execute: Handles basic execution tasks', () => {
720720

721721
const result = await execute(schema, ast, data);
722722
expect(result).to.deep.equal({
723-
errors: [
724-
{
725-
message: 'Must provide an operation.',
726-
locations: undefined,
727-
path: undefined,
728-
},
729-
],
723+
errors: [{ message: 'Must provide an operation.' }],
730724
});
731725
});
732726

@@ -748,10 +742,7 @@ describe('Execute: Handles basic execution tasks', () => {
748742
errors: [
749743
{
750744
message:
751-
'Must provide operation name if query contains ' +
752-
'multiple operations.',
753-
locations: undefined,
754-
path: undefined,
745+
'Must provide operation name if query contains multiple operations.',
755746
},
756747
],
757748
});
@@ -775,13 +766,7 @@ describe('Execute: Handles basic execution tasks', () => {
775766
operationName: 'UnknownExample',
776767
});
777768
expect(result).to.deep.equal({
778-
errors: [
779-
{
780-
message: 'Unknown operation named "UnknownExample".',
781-
locations: undefined,
782-
path: undefined,
783-
},
784-
],
769+
errors: [{ message: 'Unknown operation named "UnknownExample".' }],
785770
});
786771
});
787772

@@ -1046,17 +1031,19 @@ describe('Execute: Handles basic execution tasks', () => {
10461031
};
10471032
const result = await execute(schema, query, value);
10481033

1049-
expect(result.data).to.deep.equal({
1050-
specials: [{ value: 'foo' }, null],
1051-
});
1052-
expect(result.errors).to.have.lengthOf(1);
1053-
expect(result.errors).to.containSubset([
1054-
{
1055-
message:
1056-
'Expected value of type "SpecialType" but got: [object Object].',
1057-
locations: [{ line: 1, column: 3 }],
1034+
expect(result).to.deep.equal({
1035+
data: {
1036+
specials: [{ value: 'foo' }, null],
10581037
},
1059-
]);
1038+
errors: [
1039+
{
1040+
message:
1041+
'Expected value of type "SpecialType" but got: [object Object].',
1042+
locations: [{ line: 1, column: 3 }],
1043+
path: ['specials', 1],
1044+
},
1045+
],
1046+
});
10601047
});
10611048

10621049
it('executes ignoring invalid non-executable definitions', async () => {

‎src/execution/__tests__/lists-test.js

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
import { expect } from 'chai';
99
import { describe, it } from 'mocha';
10-
import { formatError } from '../../error';
1110
import { execute } from '../execute';
1211
import { parse } from '../../language';
1312
import {
@@ -48,18 +47,7 @@ function check(testType, testData, expected) {
4847
const ast = parse('{ nest { test } }');
4948

5049
const response = await execute(schema, ast, data);
51-
52-
// Formatting errors for ease of test writing.
53-
let result;
54-
if (response.errors) {
55-
result = {
56-
data: response.data,
57-
errors: response.errors.map(formatError),
58-
};
59-
} else {
60-
result = response;
61-
}
62-
expect(result).to.deep.equal(expected);
50+
expect(response).to.deep.equal(expected);
6351
};
6452
}
6553

‎src/execution/__tests__/mutations-test.js

Lines changed: 21 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -167,33 +167,27 @@ describe('Execute: Handles mutation execution ordering', () => {
167167

168168
const result = await execute(schema, parse(doc), new Root(6));
169169

170-
expect(result.data).to.deep.equal({
171-
first: {
172-
theNumber: 1,
173-
},
174-
second: {
175-
theNumber: 2,
176-
},
177-
third: null,
178-
fourth: {
179-
theNumber: 4,
180-
},
181-
fifth: {
182-
theNumber: 5,
183-
},
184-
sixth: null,
170+
expect(result).to.deep.equal({
171+
data: {
172+
first: { theNumber: 1 },
173+
second: { theNumber: 2 },
174+
third: null,
175+
fourth: { theNumber: 4 },
176+
fifth: { theNumber: 5 },
177+
sixth: null,
178+
},
179+
errors: [
180+
{
181+
message: 'Cannot change the number',
182+
locations: [{ line: 8, column: 7 }],
183+
path: ['third'],
184+
},
185+
{
186+
message: 'Cannot change the number',
187+
locations: [{ line: 17, column: 7 }],
188+
path: ['sixth'],
189+
},
190+
],
185191
});
186-
187-
expect(result.errors).to.have.length(2);
188-
expect(result.errors).to.containSubset([
189-
{
190-
message: 'Cannot change the number',
191-
locations: [{ line: 8, column: 7 }],
192-
},
193-
{
194-
message: 'Cannot change the number',
195-
locations: [{ line: 17, column: 7 }],
196-
},
197-
]);
198192
});
199193
});

‎src/execution/__tests__/sync-test.js

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { describe, it } from 'mocha';
1010
import { graphqlSync } from '../../graphql';
1111
import { execute } from '../execute';
1212
import { parse } from '../../language';
13+
import { validate } from '../../validation/validate';
1314
import { GraphQLSchema, GraphQLObjectType, GraphQLString } from '../../type';
1415

1516
describe('Execute: synchronously when possible', () => {
@@ -52,13 +53,7 @@ describe('Execute: synchronously when possible', () => {
5253
rootValue: 'rootValue',
5354
});
5455
expect(result).to.deep.equal({
55-
errors: [
56-
{
57-
message: 'Must provide an operation.',
58-
locations: undefined,
59-
path: undefined,
60-
},
61-
],
56+
errors: [{ message: 'Must provide an operation.' }],
6257
});
6358
});
6459

@@ -102,7 +97,7 @@ describe('Execute: synchronously when possible', () => {
10297
schema,
10398
source: doc,
10499
});
105-
expect(result).to.containSubset({
100+
expect(result).to.deep.equal({
106101
errors: [
107102
{
108103
message: 'Syntax Error: Expected Name, found {',
@@ -114,20 +109,12 @@ describe('Execute: synchronously when possible', () => {
114109

115110
it('does not return a Promise for validation errors', () => {
116111
const doc = 'fragment Example on Query { unknownField }';
112+
const validationErrors = validate(schema, parse(doc));
117113
const result = graphqlSync({
118114
schema,
119115
source: doc,
120116
});
121-
expect(result).to.containSubset({
122-
errors: [
123-
{
124-
message:
125-
'Cannot query field "unknownField" on type "Query". Did you ' +
126-
'mean "syncField" or "asyncField"?',
127-
locations: [{ line: 1, column: 29 }],
128-
},
129-
],
130-
});
117+
expect(result).to.deep.equal({ errors: validationErrors });
131118
});
132119

133120
it('does not return a Promise for sync execution', () => {

0 commit comments

Comments
 (0)