Skip to content

Error: make 'path' and 'locations' enumerable only if present #1305

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 6, 2018
Merged

Error: make 'path' and 'locations' enumerable only if present #1305

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a416de5
Error: make 'path' and 'locations' enumerable only if present
IvanGoncharov Mar 8, 2018
46f9f09
expect subset of errors
IvanGoncharov Apr 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/error/GraphQLError.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ export function GraphQLError( // eslint-disable-line no-redeclare
// By being enumerable, JSON.stringify will include `locations` in the
// resulting output. This ensures that the simplest possible GraphQL
// service adheres to the spec.
enumerable: true,
enumerable: Boolean(_locations),
},
path: {
// Coercing falsey values to undefined ensures they will not be included
Expand All @@ -160,7 +160,7 @@ export function GraphQLError( // eslint-disable-line no-redeclare
// 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: true,
enumerable: Boolean(path),
},
nodes: {
value: _nodes || undefined,
Expand Down
187 changes: 87 additions & 100 deletions src/execution/__tests__/executor-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import { expect } from 'chai';
import { describe, it } from 'mocha';
import { execute } from '../execute';
import { formatError } from '../../error';
import { parse } from '../../language';
import {
GraphQLSchema,
Expand Down Expand Up @@ -461,78 +460,79 @@ describe('Execute: Handles basic execution tasks', () => {

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

expect(result.data).to.deep.equal({
sync: 'sync',
syncError: null,
syncRawError: null,
syncReturnError: null,
syncReturnErrorList: ['sync0', null, 'sync2', null],
async: 'async',
asyncReject: null,
asyncRawReject: null,
asyncEmptyReject: null,
asyncError: null,
asyncRawError: null,
asyncReturnError: null,
});

expect(result.errors && result.errors.map(formatError)).to.deep.equal([
{
message: 'Error getting syncError',
locations: [{ line: 3, column: 7 }],
path: ['syncError'],
},
{
message: 'Error getting syncRawError',
locations: [{ line: 4, column: 7 }],
path: ['syncRawError'],
},
{
message: 'Error getting syncReturnError',
locations: [{ line: 5, column: 7 }],
path: ['syncReturnError'],
},
{
message: 'Error getting syncReturnErrorList1',
locations: [{ line: 6, column: 7 }],
path: ['syncReturnErrorList', 1],
},
{
message: 'Error getting syncReturnErrorList3',
locations: [{ line: 6, column: 7 }],
path: ['syncReturnErrorList', 3],
},
{
message: 'Error getting asyncReject',
locations: [{ line: 8, column: 7 }],
path: ['asyncReject'],
},
{
message: 'Error getting asyncRawReject',
locations: [{ line: 9, column: 7 }],
path: ['asyncRawReject'],
},
{
message: 'An unknown error occurred.',
locations: [{ line: 10, column: 7 }],
path: ['asyncEmptyReject'],
},
{
message: 'Error getting asyncError',
locations: [{ line: 11, column: 7 }],
path: ['asyncError'],
},
{
message: 'Error getting asyncRawError',
locations: [{ line: 12, column: 7 }],
path: ['asyncRawError'],
},
{
message: 'Error getting asyncReturnError',
locations: [{ line: 13, column: 7 }],
path: ['asyncReturnError'],
expect(result).to.deep.equal({
data: {
sync: 'sync',
syncError: null,
syncRawError: null,
syncReturnError: null,
syncReturnErrorList: ['sync0', null, 'sync2', null],
async: 'async',
asyncReject: null,
asyncRawReject: null,
asyncEmptyReject: null,
asyncError: null,
asyncRawError: null,
asyncReturnError: null,
},
]);
errors: [
{
message: 'Error getting syncError',
locations: [{ line: 3, column: 7 }],
path: ['syncError'],
},
{
message: 'Error getting syncRawError',
locations: [{ line: 4, column: 7 }],
path: ['syncRawError'],
},
{
message: 'Error getting syncReturnError',
locations: [{ line: 5, column: 7 }],
path: ['syncReturnError'],
},
{
message: 'Error getting syncReturnErrorList1',
locations: [{ line: 6, column: 7 }],
path: ['syncReturnErrorList', 1],
},
{
message: 'Error getting syncReturnErrorList3',
locations: [{ line: 6, column: 7 }],
path: ['syncReturnErrorList', 3],
},
{
message: 'Error getting asyncReject',
locations: [{ line: 8, column: 7 }],
path: ['asyncReject'],
},
{
message: 'Error getting asyncRawReject',
locations: [{ line: 9, column: 7 }],
path: ['asyncRawReject'],
},
{
message: '',
locations: [{ line: 10, column: 7 }],
path: ['asyncEmptyReject'],
},
{
message: 'Error getting asyncError',
locations: [{ line: 11, column: 7 }],
path: ['asyncError'],
},
{
message: 'Error getting asyncRawError',
locations: [{ line: 12, column: 7 }],
path: ['asyncRawError'],
},
{
message: 'Error getting asyncReturnError',
locations: [{ line: 13, column: 7 }],
path: ['asyncReturnError'],
},
],
});
});

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

const result = await execute(schema, ast, data);
expect(result).to.deep.equal({
errors: [
{
message: 'Must provide an operation.',
locations: undefined,
path: undefined,
},
],
errors: [{ message: 'Must provide an operation.' }],
});
});

Expand All @@ -748,10 +742,7 @@ describe('Execute: Handles basic execution tasks', () => {
errors: [
{
message:
'Must provide operation name if query contains ' +
'multiple operations.',
locations: undefined,
path: undefined,
'Must provide operation name if query contains multiple operations.',
},
],
});
Expand All @@ -775,13 +766,7 @@ describe('Execute: Handles basic execution tasks', () => {
operationName: 'UnknownExample',
});
expect(result).to.deep.equal({
errors: [
{
message: 'Unknown operation named "UnknownExample".',
locations: undefined,
path: undefined,
},
],
errors: [{ message: 'Unknown operation named "UnknownExample".' }],
});
});

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

expect(result.data).to.deep.equal({
specials: [{ value: 'foo' }, null],
});
expect(result.errors).to.have.lengthOf(1);
expect(result.errors).to.containSubset([
{
message:
'Expected value of type "SpecialType" but got: [object Object].',
locations: [{ line: 1, column: 3 }],
expect(result).to.deep.equal({
data: {
specials: [{ value: 'foo' }, null],
},
]);
errors: [
{
message:
'Expected value of type "SpecialType" but got: [object Object].',
locations: [{ line: 1, column: 3 }],
path: ['specials', 1],
},
],
});
});

it('executes ignoring invalid non-executable definitions', async () => {
Expand Down
14 changes: 1 addition & 13 deletions src/execution/__tests__/lists-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

import { expect } from 'chai';
import { describe, it } from 'mocha';
import { formatError } from '../../error';
import { execute } from '../execute';
import { parse } from '../../language';
import {
Expand Down Expand Up @@ -48,18 +47,7 @@ function check(testType, testData, expected) {
const ast = parse('{ nest { test } }');

const response = await execute(schema, ast, data);

// Formatting errors for ease of test writing.
let result;
if (response.errors) {
result = {
data: response.data,
errors: response.errors.map(formatError),
};
} else {
result = response;
}
expect(result).to.deep.equal(expected);
expect(response).to.deep.equal(expected);
};
}

Expand Down
48 changes: 21 additions & 27 deletions src/execution/__tests__/mutations-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,33 +167,27 @@ describe('Execute: Handles mutation execution ordering', () => {

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

expect(result.data).to.deep.equal({
first: {
theNumber: 1,
},
second: {
theNumber: 2,
},
third: null,
fourth: {
theNumber: 4,
},
fifth: {
theNumber: 5,
},
sixth: null,
expect(result).to.deep.equal({
data: {
first: { theNumber: 1 },
second: { theNumber: 2 },
third: null,
fourth: { theNumber: 4 },
fifth: { theNumber: 5 },
sixth: null,
},
errors: [
{
message: 'Cannot change the number',
locations: [{ line: 8, column: 7 }],
path: ['third'],
},
{
message: 'Cannot change the number',
locations: [{ line: 17, column: 7 }],
path: ['sixth'],
},
],
});

expect(result.errors).to.have.length(2);
expect(result.errors).to.containSubset([
{
message: 'Cannot change the number',
locations: [{ line: 8, column: 7 }],
},
{
message: 'Cannot change the number',
locations: [{ line: 17, column: 7 }],
},
]);
});
});
23 changes: 5 additions & 18 deletions src/execution/__tests__/sync-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { describe, it } from 'mocha';
import { graphqlSync } from '../../graphql';
import { execute } from '../execute';
import { parse } from '../../language';
import { validate } from '../../validation/validate';
import { GraphQLSchema, GraphQLObjectType, GraphQLString } from '../../type';

describe('Execute: synchronously when possible', () => {
Expand Down Expand Up @@ -52,13 +53,7 @@ describe('Execute: synchronously when possible', () => {
rootValue: 'rootValue',
});
expect(result).to.deep.equal({
errors: [
{
message: 'Must provide an operation.',
locations: undefined,
path: undefined,
},
],
errors: [{ message: 'Must provide an operation.' }],
});
});

Expand Down Expand Up @@ -102,7 +97,7 @@ describe('Execute: synchronously when possible', () => {
schema,
source: doc,
});
expect(result).to.containSubset({
expect(result).to.deep.equal({
errors: [
{
message: 'Syntax Error: Expected Name, found {',
Expand All @@ -114,20 +109,12 @@ describe('Execute: synchronously when possible', () => {

it('does not return a Promise for validation errors', () => {
const doc = 'fragment Example on Query { unknownField }';
const validationErrors = validate(schema, parse(doc));
const result = graphqlSync({
schema,
source: doc,
});
expect(result).to.containSubset({
errors: [
{
message:
'Cannot query field "unknownField" on type "Query". Did you ' +
'mean "syncField" or "asyncField"?',
locations: [{ line: 1, column: 29 }],
},
],
});
expect(result).to.deep.equal({ errors: validationErrors });
});

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