Skip to content

Errors thrown from resolvers have the execution path #396

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.

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 10 commits into from
Jun 10, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 15 additions & 0 deletions src/__tests__/starWarsIntrospection-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,13 @@ describe('Star Wars Introspection Tests', () => {
kind: 'LIST'
}
},
{
name: 'secretBackstory',
type: {
name: 'String',
kind: 'SCALAR'
}
},
{
name: 'primaryFunction',
type: {
Expand Down Expand Up @@ -284,6 +291,14 @@ describe('Star Wars Introspection Tests', () => {
}
}
},
{
name: 'secretBackstory',
type: {
name: 'String',
kind: 'SCALAR',
ofType: null
}
},
{
name: 'primaryFunction',
type: {
Expand Down
164 changes: 164 additions & 0 deletions src/__tests__/starWarsQuery-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ import { expect } from 'chai';
import { describe, it } from 'mocha';
import { StarWarsSchema } from './starWarsSchema.js';
import { graphql } from '../graphql';
import {
GraphQLObjectType,
GraphQLNonNull,
GraphQLSchema,
GraphQLString,
} from '../type';

// 80+ char lines are useful in describe/it, so ignore in this file.
/* eslint-disable max-len */
Expand Down Expand Up @@ -364,4 +370,162 @@ describe('Star Wars Query Tests', () => {
expect(result).to.deep.equal({ data: expected });
});
});

describe('Reporting errors raised in resolvers', () => {
it('Correctly reports error on accessing secretBackstory', async () => {
const query = `
query HeroNameQuery {
hero {
name
secretBackstory
}
}
`;
const expected = {
hero: {
name: 'R2-D2',
secretBackstory: null
}
};
const expectedErrors = [ 'secretBackstory is secret.' ];
const result = await graphql(StarWarsSchema, query);
expect(result.data).to.deep.equal(expected);
expect(result.errors.map(e => e.message)).to.deep.equal(expectedErrors);
expect(
result.errors.map(e => e.path)).to.deep.equal(
[ [ 'hero', 'secretBackstory' ] ]);
});

it('Correctly reports error on accessing secretBackstory in a list', async () => {
const query = `
query HeroNameQuery {
hero {
name
friends {
name
secretBackstory
}
}
}
`;
const expected = {
hero: {
name: 'R2-D2',
friends: [
{
name: 'Luke Skywalker',
secretBackstory: null,
},
{
name: 'Han Solo',
secretBackstory: null,
},
{
name: 'Leia Organa',
secretBackstory: null,
},
]
}
};
const expectedErrors = [
'secretBackstory is secret.',
'secretBackstory is secret.',
'secretBackstory is secret.',
];
const result = await graphql(StarWarsSchema, query);
expect(result.data).to.deep.equal(expected);
expect(result.errors.map(e => e.message)).to.deep.equal(expectedErrors);
expect(
result.errors.map(e => e.path)
).to.deep.equal(
[
[ 'hero', 'friends', 0, 'secretBackstory' ],
[ 'hero', 'friends', 1, 'secretBackstory' ],
[ 'hero', 'friends', 2, 'secretBackstory' ],
]);
});

it('Correctly reports error on accessing through an alias', async () => {
const query = `
query HeroNameQuery {
mainHero: hero {
name
story: secretBackstory
}
}
`;
const expected = {
mainHero: {
name: 'R2-D2',
story: null,
}
};
const expectedErrors = [
'secretBackstory is secret.',
];
const result = await graphql(StarWarsSchema, query);
expect(result.data).to.deep.equal(expected);
expect(result.errors.map(e => e.message)).to.deep.equal(expectedErrors);
expect(
result.errors.map(e => e.path)
).to.deep.equal([ [ 'mainHero', 'story' ] ]);
});

it('Full response path is included when fields are non-nullable', async () => {
const A = new GraphQLObjectType({
name: 'A',
fields: () => ({
nullableA: {
type: A,
resolve: () => ({}),
},
nonNullA: {
type: new GraphQLNonNull(A),
resolve: () => ({}),
},
throws: {
type: new GraphQLNonNull(GraphQLString),
resolve: () => { throw new Error('Catch me if you can'); },
},
}),
});
const queryType = new GraphQLObjectType({
name: 'query',
fields: () => ({
nullableA: {
type: A,
resolve: () => ({})
}
}),
});
const schema = new GraphQLSchema({
query: queryType,
});

const query = `
query {
nullableA {
nullableA {
nonNullA {
nonNullA {
throws
}
}
}
}
}
`;

const result = await graphql(schema, query);
const expected = {
nullableA: {
nullableA: null
}
};
expect(result.data).to.deep.equal(expected);
expect(
result.errors.map(e => e.path)).to.deep.equal(
[ [ 'nullableA', 'nullableA', 'nonNullA', 'nonNullA', 'throws' ] ]);
});
});
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another important test to write, which you will need to write a custom schema for, is that the appropriate response path is included when fields are non-nullable:

type A {
  nullableA: A
  nonNullA: A!
  throws: String!
}

query {
  nullableA {
    nullableA {
      nonNullA { 
        nonNullA { 
          throws
        }
      }
    }
  }
}

Should provide the response:

{
  data: {
    nullableA: {
      nullableA: null
    }
  },
  errors: [
    {
       path: [ 'nullableA', 'nullableA', 'nonNullA', 'nonNullA', 'throws' ]
    }
  ]
}

21 changes: 21 additions & 0 deletions src/__tests__/starWarsSchema.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ const episodeEnum = new GraphQLEnumType({
* name: String
* friends: [Character]
* appearsIn: [Episode]
* secretBackstory: String
* }
*/
const characterInterface = new GraphQLInterfaceType({
Expand All @@ -125,6 +126,10 @@ const characterInterface = new GraphQLInterfaceType({
type: new GraphQLList(episodeEnum),
description: 'Which movies they appear in.',
},
secretBackstory: {
type: GraphQLString,
description: 'All secrets about their past.',
},
}),
resolveType: character => {
return getHuman(character.id) ? humanType : droidType;
Expand All @@ -140,6 +145,7 @@ const characterInterface = new GraphQLInterfaceType({
* name: String
* friends: [Character]
* appearsIn: [Episode]
* secretBackstory: String
* }
*/
const humanType = new GraphQLObjectType({
Expand Down Expand Up @@ -168,6 +174,13 @@ const humanType = new GraphQLObjectType({
type: GraphQLString,
description: 'The home planet of the human, or null if unknown.',
},
secretBackstory: {
type: GraphQLString,
description: 'Where are they from and how they came to be who they are.',
resolve: () => {
throw new Error('secretBackstory is secret.');
},
},
}),
interfaces: [ characterInterface ]
});
Expand All @@ -181,6 +194,7 @@ const humanType = new GraphQLObjectType({
* name: String
* friends: [Character]
* appearsIn: [Episode]
* secretBackstory: String
* primaryFunction: String
* }
*/
Expand All @@ -206,6 +220,13 @@ const droidType = new GraphQLObjectType({
type: new GraphQLList(episodeEnum),
description: 'Which movies they appear in.',
},
secretBackstory: {
type: GraphQLString,
description: 'Construction date and the name of the designer.',
resolve: () => {
throw new Error('secretBackstory is secret.');
},
},
primaryFunction: {
type: GraphQLString,
description: 'The primary function of the droid.',
Expand Down
1 change: 1 addition & 0 deletions src/error/GraphQLError.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export class GraphQLError extends Error {
source: Source;
positions: Array<number>;
locations: any;
path: Array<string | number>;
originalError: ?Error;

constructor(
Expand Down
4 changes: 3 additions & 1 deletion src/error/locatedError.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,15 @@ import { GraphQLError } from './GraphQLError';
*/
export function locatedError(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the right change here is to add a third path argument to locatedError

originalError: ?Error,
nodes: Array<any>
nodes: Array<any>,
path: Array<string | number>
): GraphQLError {
const message = originalError ?
originalError.message || String(originalError) :
'An unknown error occurred.';
const stack = originalError ? originalError.stack : null;
const error = new GraphQLError(message, nodes, stack);
error.originalError = originalError;
error.path = path;
return error;
}
Loading