Skip to content

Commit

Permalink
Add message and locations as enumerable properties on GraphQLError.
Browse files Browse the repository at this point in the history 
As brought up in #426, it can be confusing to return instances of `Error` from the `graphql` function since the expectation is that you will call `JSON.stringify` and submit the result to a client. Calling `JSON.stringify` on an instance of `Error` results in `{}` since it has no enumerable properties.

In order to have behavior that matches the GraphQL spec easier to achieve, this makes these two properties enumerable. I think this will be the best of both worlds - anyone who wishes the operate directly on the Error instances will still have the ability to do so (for example, to access the `stack` property), but it can still be converted to JSON directly and get a spec-compliant result.
  • Loading branch information
@leebyron
leebyron committed Jul 19, 2016
1 parent b080bf5 commit 1a48021
Show file tree
Hide file tree
Showing 7 changed files with 94 additions and 62 deletions.
1 change: 1 addition & 0 deletions package.json
View file
Expand Up @@ -59,6 +59,7 @@
"babel-plugin-transform-object-rest-spread": "6.8.0",
"babel-plugin-transform-regenerator": "6.9.0",
"chai": "3.5.0",
"chai-json-equal": "0.0.1",
"chai-subset": "1.2.2",
"coveralls": "2.11.9",
"eslint": "2.11.1",
Expand Down
5 changes: 2 additions & 3 deletions resources/mocha-bootload.js
View file
Expand Up @@ -8,9 +8,8 @@
*/

var chai = require('chai');

var chaiSubset = require('chai-subset');
chai.use(chaiSubset);
chai.use(require('chai-subset'));
chai.use(require('chai-json-equal'));

process.on('unhandledRejection', function (error) {
console.error('Unhandled Promise Rejection:');
Expand Down
25 changes: 21 additions & 4 deletions src/error/GraphQLError.js
View file
Expand Up @@ -32,7 +32,17 @@ export class GraphQLError extends Error {
positions?: Array<number>
) {
super(message);
this.message = message;

Object.defineProperty(this, 'message', {
value: message,
// By being enumerable, JSON.stringify will include `message` in the
// resulting output. This ensures that the simplist possible GraphQL
// service adheres to the spec.
enumerable: true,
// Note: you really shouldn't overwrite message, but it enables
// Error brand-checking.
writable: true,
});

Object.defineProperty(this, 'stack', {
value: stack || message,
Expand All @@ -41,6 +51,7 @@ export class GraphQLError extends Error {
// if stack is a writable property.
writable: true,
});

Object.defineProperty(this, 'nodes', { value: nodes });

// Note: flow does not yet know about Object.defineProperty with `get`.
Expand Down Expand Up @@ -72,10 +83,16 @@ export class GraphQLError extends Error {

Object.defineProperty(this, 'locations', ({
get() {
if (this.positions && this.source) {
return this.positions.map(pos => getLocation(this.source, pos));
const _positions = this.positions;
const _source = this.source;
if (_positions && _positions.length > 0 && _source) {
return _positions.map(pos => getLocation(_source, pos));
}
}
},
// By being enumerable, JSON.stringify will include `locations` in the
// resulting output. This ensures that the simplist possible GraphQL
// service adheres to the spec.
enumerable: true,
}: any));
}
}
21 changes: 12 additions & 9 deletions src/execution/__tests__/abstract-test.js
View file
Expand Up @@ -20,7 +20,6 @@ import {
GraphQLBoolean,
} from '../../';


class Dog {
constructor(name, woofs) {
this.name = name;
Expand Down Expand Up @@ -245,7 +244,7 @@ describe('Execute: Handles execution of abstract types', () => {

const result = await graphql(schema, query);

expect(result).to.deep.equal({
expect(result).to.jsonEqual({
data: {
pets: [
{ name: 'Odie',
Expand All @@ -256,9 +255,11 @@ describe('Execute: Handles execution of abstract types', () => {
]
},
errors: [
new Error(
'Runtime Object type "Human" is not a possible type for "Pet".'
)
{
message:
'Runtime Object type "Human" is not a possible type for "Pet".',
locations: [ { line: 2, column: 7 } ]
}
]
});
});
Expand Down Expand Up @@ -332,7 +333,7 @@ describe('Execute: Handles execution of abstract types', () => {

const result = await graphql(schema, query);

expect(result).to.deep.equal({
expect(result).to.jsonEqual({
data: {
pets: [
{ name: 'Odie',
Expand All @@ -343,9 +344,11 @@ describe('Execute: Handles execution of abstract types', () => {
]
},
errors: [
new Error(
'Runtime Object type "Human" is not a possible type for "Pet".'
)
{
message:
'Runtime Object type "Human" is not a possible type for "Pet".',
locations: [ { line: 2, column: 7 } ]
}
]
});
});
Expand Down
10 changes: 5 additions & 5 deletions src/execution/__tests__/executor-test.js
View file
Expand Up @@ -787,11 +787,11 @@ describe('Execute: Handles basic execution tasks', () => {
caughtError = error;
}

expect(caughtError).to.deep.equal(
new Error(
'GraphQL cannot execute a request containing a ObjectTypeDefinition.'
)
);
expect(caughtError).to.jsonEqual({
message:
'GraphQL cannot execute a request containing a ObjectTypeDefinition.',
locations: [ { line: 4, column: 7 } ]
});
});

});
2 changes: 1 addition & 1 deletion src/execution/execute.js
View file
Expand Up @@ -192,7 +192,7 @@ function buildExecutionContext(
break;
default: throw new GraphQLError(
`GraphQL cannot execute a request containing a ${definition.kind}.`,
definition
[ definition ]
);
}
});
Expand Down
92 changes: 52 additions & 40 deletions src/type/__tests__/enumType-test.js
View file
Expand Up @@ -129,7 +129,7 @@ describe('Type System: Enum Values', () => {
it('accepts enum literals as input', async () => {
expect(
await graphql(schema, '{ colorInt(fromEnum: GREEN) }')
).to.deep.equal({
).to.jsonEqual({
data: {
colorInt: 1
}
Expand All @@ -139,7 +139,7 @@ describe('Type System: Enum Values', () => {
it('enum may be output type', async () => {
expect(
await graphql(schema, '{ colorEnum(fromInt: 1) }')
).to.deep.equal({
).to.jsonEqual({
data: {
colorEnum: 'GREEN'
}
Expand All @@ -149,7 +149,7 @@ describe('Type System: Enum Values', () => {
it('enum may be both input and output type', async () => {
expect(
await graphql(schema, '{ colorEnum(fromEnum: GREEN) }')
).to.deep.equal({
).to.jsonEqual({
data: {
colorEnum: 'GREEN'
}
Expand All @@ -159,20 +159,22 @@ describe('Type System: Enum Values', () => {
it('does not accept string literals', async () => {
expect(
await graphql(schema, '{ colorEnum(fromEnum: "GREEN") }')
).to.deep.equal({
).to.jsonEqual({
errors: [
new Error(
'Argument "fromEnum" has invalid value "GREEN".' +
'\nExpected type \"Color\", found "GREEN".'
)
{
message:
'Argument "fromEnum" has invalid value "GREEN".' +
'\nExpected type \"Color\", found "GREEN".',
locations: [ { line: 1, column: 23 } ]
}
]
});
});

it('does not accept incorrect internal value', async () => {
expect(
await graphql(schema, '{ colorEnum(fromString: "GREEN") }')
).to.deep.equal({
).to.jsonEqual({
data: {
colorEnum: null
}
Expand All @@ -182,25 +184,29 @@ describe('Type System: Enum Values', () => {
it('does not accept internal value in place of enum literal', async () => {
expect(
await graphql(schema, '{ colorEnum(fromEnum: 1) }')
).to.deep.equal({
).to.jsonEqual({
errors: [
new Error(
'Argument "fromEnum" has invalid value 1.' +
'\nExpected type "Color", found 1.'
)
{
message:
'Argument "fromEnum" has invalid value 1.' +
'\nExpected type "Color", found 1.',
locations: [ { line: 1, column: 23 } ]
}
]
});
});

it('does not accept enum literal in place of int', async () => {
expect(
await graphql(schema, '{ colorEnum(fromInt: GREEN) }')
).to.deep.equal({
).to.jsonEqual({
errors: [
new Error(
'Argument "fromInt" has invalid value GREEN.' +
'\nExpected type "Int", found GREEN.'
)
{
message:
'Argument "fromInt" has invalid value GREEN.' +
'\nExpected type "Int", found GREEN.',
locations: [ { line: 1, column: 22 } ]
}
]
});
});
Expand All @@ -214,7 +220,7 @@ describe('Type System: Enum Values', () => {
null,
{ color: 'BLUE' }
)
).to.deep.equal({
).to.jsonEqual({
data: {
colorEnum: 'BLUE'
}
Expand All @@ -230,7 +236,7 @@ describe('Type System: Enum Values', () => {
null,
{ color: 'GREEN' }
)
).to.deep.equal({
).to.jsonEqual({
data: {
favoriteEnum: 'GREEN'
}
Expand All @@ -246,7 +252,7 @@ describe('Type System: Enum Values', () => {
null,
{ color: 'GREEN' }
)
).to.deep.equal({
).to.jsonEqual({
data: {
subscribeToEnum: 'GREEN'
}
Expand All @@ -262,12 +268,14 @@ describe('Type System: Enum Values', () => {
null,
{ color: 2 }
)
).to.deep.equal({
).to.jsonEqual({
errors: [
new Error(
'Variable "\$color" got invalid value 2.' +
'\nExpected type "Color", found 2.'
)
{
message:
'Variable "\$color" got invalid value 2.' +
'\nExpected type "Color", found 2.',
locations: [ { line: 1, column: 12 } ]
}
]
});
});
Expand All @@ -281,12 +289,14 @@ describe('Type System: Enum Values', () => {
null,
{ color: 'BLUE' }
)
).to.deep.equal({
).to.jsonEqual({
errors: [
new Error(
'Variable "$color" of type "String!" used in position ' +
'expecting type "Color".'
)
{
message:
'Variable "$color" of type "String!" used in position ' +
'expecting type "Color".',
locations: [ { line: 1, column: 12 }, { line: 1, column: 51 } ]
}
]
});
});
Expand All @@ -300,12 +310,14 @@ describe('Type System: Enum Values', () => {
null,
{ color: 2 }
)
).to.deep.equal({
).to.jsonEqual({
errors: [
new Error(
'Variable "$color" of type "Int!" used in position ' +
'expecting type "Color".'
)
{
message:
'Variable "$color" of type "Int!" used in position ' +
'expecting type "Color".',
locations: [ { line: 1, column: 12 }, { line: 1, column: 48 } ]
}
]
});
});
Expand All @@ -316,7 +328,7 @@ describe('Type System: Enum Values', () => {
colorEnum(fromEnum: RED)
colorInt(fromEnum: RED)
}`)
).to.deep.equal({
).to.jsonEqual({
data: {
colorEnum: 'RED',
colorInt: 0
Expand All @@ -330,7 +342,7 @@ describe('Type System: Enum Values', () => {
colorEnum
colorInt
}`)
).to.deep.equal({
).to.jsonEqual({
data: {
colorEnum: null,
colorInt: null
Expand All @@ -355,7 +367,7 @@ describe('Type System: Enum Values', () => {
good: complexEnum(provideGoodValue: true)
bad: complexEnum(provideBadValue: true)
}`)
).to.deep.equal({
).to.jsonEqual({
data: {
first: 'ONE',
second: 'TWO',
Expand Down

0 comments on commit 1a48021

Please sign in to comment.