Skip to content

Commit 60d0cbb

Browse files
committed
Use error ctor for syntax errors rather than mutation
Fixes issue where syntax errors were reported with poor messages.
1 parent a4c5605 commit 60d0cbb

File tree

3 files changed

+65
-38
lines changed

3 files changed

+65
-38
lines changed

src/error/GraphQLError.js

Lines changed: 39 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -10,58 +10,64 @@
1010

1111
import { getLocation } from '../language';
1212
import type { Node } from '../language/ast';
13+
import type { Source } from '../language/source';
1314

1415

1516
export class GraphQLError extends Error {
1617
message: string;
1718
stack: string;
1819
nodes: ?Array<Node>;
19-
source: any;
20-
positions: any;
20+
source: Source;
21+
positions: Array<number>;
2122
locations: any;
2223

2324
constructor(
2425
message: string,
2526
// A flow bug keeps us from declaring nodes as an array of Node
2627
nodes?: Array<any/* Node */>,
27-
stack?: ?string
28+
stack?: ?string,
29+
source?: Source,
30+
positions?: Array<number>
2831
) {
2932
super(message);
3033
this.message = message;
34+
3135
Object.defineProperty(this, 'stack', { value: stack || message });
3236
Object.defineProperty(this, 'nodes', { value: nodes });
33-
}
34-
}
3537

36-
// Note: flow does not yet know about Object.defineProperty with `get`.
37-
Object.defineProperty(GraphQLError.prototype, 'source', ({
38-
get() {
39-
var nodes = this.nodes;
40-
if (nodes && nodes.length > 0) {
41-
var node = nodes[0];
42-
return node && node.loc && node.loc.source;
43-
}
44-
}
45-
}: any));
38+
// Note: flow does not yet know about Object.defineProperty with `get`.
39+
Object.defineProperty(this, 'source', ({
40+
get() {
41+
if (source) {
42+
return source;
43+
}
44+
if (nodes && nodes.length > 0) {
45+
var node = nodes[0];
46+
return node && node.loc && node.loc.source;
47+
}
48+
}
49+
}: any));
4650

47-
Object.defineProperty(GraphQLError.prototype, 'positions', ({
48-
get() {
49-
var nodes = this.nodes;
50-
if (nodes) {
51-
var positions = nodes.map(node => node.loc && node.loc.start);
52-
if (positions.some(p => p)) {
53-
return positions;
51+
Object.defineProperty(this, 'positions', ({
52+
get() {
53+
if (positions) {
54+
return positions;
55+
}
56+
if (nodes) {
57+
var nodePositions = nodes.map(node => node.loc && node.loc.start);
58+
if (nodePositions.some(p => p)) {
59+
return nodePositions;
60+
}
61+
}
5462
}
55-
}
56-
}
57-
}: any));
63+
}: any));
5864

59-
Object.defineProperty(GraphQLError.prototype, 'locations', ({
60-
get() {
61-
var positions = this.positions;
62-
var source = this.source;
63-
if (positions && source) {
64-
return positions.map(pos => getLocation(source, pos));
65-
}
65+
Object.defineProperty(this, 'locations', ({
66+
get() {
67+
if (this.positions && this.source) {
68+
return this.positions.map(pos => getLocation(this.source, pos));
69+
}
70+
}
71+
}: any));
6672
}
67-
}: any));
73+
}

src/error/syntaxError.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,12 @@ export function syntaxError(
2424
var location = getLocation(source, position);
2525
var error = new GraphQLError(
2626
`Syntax Error ${source.name} (${location.line}:${location.column}) ` +
27-
description + '\n\n' + highlightSourceAtLocation(source, location)
27+
description + '\n\n' + highlightSourceAtLocation(source, location),
28+
undefined,
29+
undefined,
30+
source,
31+
[ position ]
2832
);
29-
error.nodes = null;
30-
error.positions = [ position ];
31-
error.locations = [ location ];
32-
error.source = source;
3333
return error;
3434
}
3535

src/language/__tests__/parser.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,27 @@ describe('Parser', () => {
4747

4848
it('parse provides useful errors', () => {
4949

50+
var caughtError;
51+
try {
52+
parse('{');
53+
} catch (error) {
54+
caughtError = error;
55+
}
56+
57+
expect(caughtError.message).to.equal(
58+
`Syntax Error GraphQL (1:2) Expected Name, found EOF
59+
60+
1: {
61+
^
62+
`
63+
);
64+
65+
expect(caughtError.positions).to.deep.equal([ 1 ]);
66+
67+
expect(caughtError.locations).to.deep.equal([
68+
{ line: 1, column: 2 }
69+
]);
70+
5071
expect(
5172
() => parse(
5273
`{ ...MissingOn }

0 commit comments

Comments
 (0)