From e54786ddb32d51e3924be94a5ce79ae6fa53b673 Mon Sep 17 00:00:00 2001 From: Tim Ryan Date: Thu, 16 Feb 2017 15:06:35 -0500 Subject: [PATCH 1/4] Adds message to stack trace of GraphQLError. --- src/error/GraphQLError.js | 35 +++++++++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/src/error/GraphQLError.js b/src/error/GraphQLError.js index 5489a77d82..298692fde7 100644 --- a/src/error/GraphQLError.js +++ b/src/error/GraphQLError.js @@ -86,6 +86,33 @@ export function GraphQLError( // eslint-disable-line no-redeclare path?: ?Array, originalError?: ?Error ) { + // Define message so it can be captured in stack trace. + 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, + writable: true + }); + + // Include (non-enumerable) stack trace. + if (originalError && originalError.stack) { + Object.defineProperty(this, 'stack', { + value: originalError.stack, + writable: true, + configurable: true + }); + } else if (Error.captureStackTrace) { + Error.captureStackTrace(this, GraphQLError); + } else { + Object.defineProperty(this, 'stack', { + value: Error().stack, + writable: true, + configurable: true + }); + } + // Compute locations in the source for the given nodes/positions. let _source = source; if (!_source && nodes && nodes.length > 0) { @@ -109,14 +136,6 @@ export function GraphQLError( // eslint-disable-line no-redeclare } Object.defineProperties(this, { - message: { - value: message, - // By being enumerable, JSON.stringify will include `message` in the - // resulting output. This ensures that the simplest possible GraphQL - // service adheres to the spec. - enumerable: true, - writable: true - }, locations: { // Coercing falsey values to undefined ensures they will not be included // in JSON.stringify() when not provided. From e53401cde91626cd1c3d3159d48fda9ff71c7dcc Mon Sep 17 00:00:00 2001 From: Greg Hurrell Date: Thu, 16 Feb 2017 13:04:10 -0800 Subject: [PATCH 2/4] Fix typo in comment --- src/error/GraphQLError.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/error/GraphQLError.js b/src/error/GraphQLError.js index 298692fde7..05b530319a 100644 --- a/src/error/GraphQLError.js +++ b/src/error/GraphQLError.js @@ -90,7 +90,7 @@ export function GraphQLError( // eslint-disable-line no-redeclare 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 + // resulting output. This ensures that the simplest possible GraphQL // service adheres to the spec. enumerable: true, writable: true From 282b8c348dcd3b3dc40320085669eaea184cc3b2 Mon Sep 17 00:00:00 2001 From: Tim Ryan Date: Fri, 17 Feb 2017 11:51:40 -0500 Subject: [PATCH 3/4] Allows buildASTSchema to throw errors with source locations. --- src/error/syntaxError.js | 51 ++++++++++++++++--- src/jsutils/invariant.js | 5 +- src/type/definition.js | 33 ++++++++---- .../__tests__/buildASTSchema-test.js | 26 ++++++++++ src/utilities/buildASTSchema.js | 21 +++++--- 5 files changed, 111 insertions(+), 25 deletions(-) diff --git a/src/error/syntaxError.js b/src/error/syntaxError.js index d805801a06..a8752e3c4e 100644 --- a/src/error/syntaxError.js +++ b/src/error/syntaxError.js @@ -11,6 +11,29 @@ import { getLocation } from '../language/location'; import type { Source } from '../language/source'; import { GraphQLError } from './GraphQLError'; +import type { ASTNode } from '../language/ast'; + +/** + * Produces a string for formatting a syntax or validation error with an + * embedded location. + */ +export function printLocatedError( + source: Source, + position: number, + message: string, + description?: string, +): GraphQLError { + const location = getLocation(source, position); + const body = `${message} (${location.line}:${location.column})` + + (description ? ' ' + description : '') + + '\n\n' + highlightSourceAtLocation(source, location); + return new GraphQLError( + body, + undefined, + source, + [ position ] + ); +} /** * Produces a GraphQLError representing a syntax error, containing useful @@ -21,15 +44,29 @@ export function syntaxError( position: number, description: string ): GraphQLError { - const location = getLocation(source, position); - const error = new GraphQLError( - `Syntax Error ${source.name} (${location.line}:${location.column}) ` + - description + '\n\n' + highlightSourceAtLocation(source, location), - undefined, + return printLocatedError( source, - [ position ] + position, + `Syntax Error ${source.name}`, + description, ); - return error; +} + +/** + * Produces a GraphQLError for the invariant(...) function that renders the + * location where a validation error occurred. If no source is passed in, it + * returns an error containing the message, without context. + */ +export function validationError( + message: string, + node?: ASTNode, + source?: Source +): GraphQLError { + const position = node ? (node.loc ? node.loc.start : null) : null; + if (position == null || source == null) { + return new GraphQLError(message); + } + return printLocatedError(source, position, `Validation Error: ${message}`); } /** diff --git a/src/jsutils/invariant.js b/src/jsutils/invariant.js index d88eb29705..96e02289bd 100644 --- a/src/jsutils/invariant.js +++ b/src/jsutils/invariant.js @@ -8,8 +8,11 @@ * of patent rights can be found in the PATENTS file in the same directory. */ -export default function invariant(condition: mixed, message: string) { +export default function invariant(condition: mixed, message: string | Error) { if (!condition) { + if (message instanceof Error) { + throw message; + } throw new Error(message); } } diff --git a/src/type/definition.js b/src/type/definition.js index 434b0c99be..685b7a15ef 100644 --- a/src/type/definition.js +++ b/src/type/definition.js @@ -19,7 +19,10 @@ import type { ValueNode, } from '../language/ast'; import type { GraphQLSchema } from './schema'; +import { validationError } from '../error/syntaxError'; +import type { Source } from '../language/source'; +import type { ASTNode } from '../language/ast'; // Predicates & Assertions @@ -82,11 +85,16 @@ export function isInputType(type: ?GraphQLType): boolean %checks { ); } -export function assertInputType(type: ?GraphQLType): GraphQLInputType { - invariant( - isInputType(type), - `Expected ${String(type)} to be a GraphQL input type.` - ); +export function assertInputType( + type: ?GraphQLType, + typeNode?: ASTNode, + source?: Source +): GraphQLInputType { + invariant(isInputType(type), + validationError( + `Expected ${String(type)} to be a GraphQL input type.`, + typeNode, + source)); return type; } @@ -121,11 +129,16 @@ export function isOutputType(type: ?GraphQLType): boolean %checks { ); } -export function assertOutputType(type: ?GraphQLType): GraphQLOutputType { - invariant( - isOutputType(type), - `Expected ${String(type)} to be a GraphQL output type.`, - ); +export function assertOutputType( + type: ?GraphQLType, + typeNode?: ASTNode, + source?: Source +): GraphQLOutputType { + invariant(isOutputType(type), + validationError( + `Expected ${String(type)} to be a GraphQL output type.`, + typeNode, + source)); return type; } diff --git a/src/utilities/__tests__/buildASTSchema-test.js b/src/utilities/__tests__/buildASTSchema-test.js index c80a3d15df..fdf6cb64db 100644 --- a/src/utilities/__tests__/buildASTSchema-test.js +++ b/src/utilities/__tests__/buildASTSchema-test.js @@ -17,6 +17,7 @@ import { GraphQLSkipDirective, GraphQLIncludeDirective, GraphQLDeprecatedDirective, + Source, } from '../../'; /** @@ -819,5 +820,30 @@ type Repeated { const doc = parse(body); expect(() => buildASTSchema(doc)) .to.throw('Type "Repeated" was defined more than once.'); + + it('warns with a location where validation errors occur', () => { + const body = new Source(`type OutputType { + value: String + } + + input InputType { + value: String + } + + type Query { + output: [OutputType] + } + + type Mutation { + signup (password: OutputType): OutputType + }`); + const doc = parse(body); + expect(() => buildASTSchema(doc, body)) + .to.throw(`GraphQLError: Validation Error: Expected Input type (14:25) + +13: type Mutation { +14: signup (password: OutputType): OutputType + ^ +15: }`); }); }); diff --git a/src/utilities/buildASTSchema.js b/src/utilities/buildASTSchema.js index 7b6ef88b74..c4fcaf8c96 100644 --- a/src/utilities/buildASTSchema.js +++ b/src/utilities/buildASTSchema.js @@ -14,8 +14,9 @@ import keyValMap from '../jsutils/keyValMap'; import { valueFromAST } from './valueFromAST'; import { TokenKind } from '../language/lexer'; import { parse } from '../language/parser'; -import type { Source } from '../language/source'; +import { Source } from '../language/source'; import { getArgumentValues } from '../execution/values'; +import { validationError } from '../error/syntaxError'; import { LIST_TYPE, @@ -135,7 +136,10 @@ function getNamedTypeNode(typeNode: TypeNode): NamedTypeNode { * Given that AST it constructs a GraphQLSchema. The resulting schema * has no resolve methods, so execution will use default resolvers. */ -export function buildASTSchema(ast: DocumentNode): GraphQLSchema { +export function buildASTSchema( + ast: DocumentNode, + source?: Source +): GraphQLSchema { if (!ast || ast.kind !== DOCUMENT) { throw new Error('Must provide a document ast.'); } @@ -303,22 +307,24 @@ export function buildASTSchema(ast: DocumentNode): GraphQLSchema { } function produceInputType(typeNode: TypeNode): GraphQLInputType { - return assertInputType(produceType(typeNode)); + return assertInputType(produceType(typeNode), typeNode, source); } function produceOutputType(typeNode: TypeNode): GraphQLOutputType { - return assertOutputType(produceType(typeNode)); + return assertOutputType(produceType(typeNode), typeNode, source); } function produceObjectType(typeNode: TypeNode): GraphQLObjectType { const type = produceType(typeNode); - invariant(type instanceof GraphQLObjectType, 'Expected Object type.'); + invariant(type instanceof GraphQLObjectType, + validationError('Expected Object type', typeNode, source)); return type; } function produceInterfaceType(typeNode: TypeNode): GraphQLInterfaceType { const type = produceType(typeNode); - invariant(type instanceof GraphQLInterfaceType, 'Expected Interface type.'); + invariant(type instanceof GraphQLInterfaceType, + validationError('Expected Interface type', typeNode, source)); return type; } @@ -524,7 +530,8 @@ export function getDescription(node: { loc?: Location }): ?string { * document. */ export function buildSchema(source: string | Source): GraphQLSchema { - return buildASTSchema(parse(source)); + const sourceObj = typeof source === 'string' ? new Source(source) : source; + return buildASTSchema(parse(sourceObj), sourceObj); } // Count the number of spaces on the starting side of a string. From e881215982d44945b348031218b1b0255d7d9ec6 Mon Sep 17 00:00:00 2001 From: Tim Ryan Date: Tue, 21 Mar 2017 16:29:02 -0400 Subject: [PATCH 4/4] Corrects argument order in validationError. --- src/error/syntaxError.js | 4 ++-- src/type/definition.js | 8 ++++---- src/utilities/buildASTSchema.js | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/error/syntaxError.js b/src/error/syntaxError.js index a8752e3c4e..01e2f220e5 100644 --- a/src/error/syntaxError.js +++ b/src/error/syntaxError.js @@ -58,9 +58,9 @@ export function syntaxError( * returns an error containing the message, without context. */ export function validationError( + source: ?Source, + node: ?ASTNode, message: string, - node?: ASTNode, - source?: Source ): GraphQLError { const position = node ? (node.loc ? node.loc.start : null) : null; if (position == null || source == null) { diff --git a/src/type/definition.js b/src/type/definition.js index 685b7a15ef..cc8ea631ab 100644 --- a/src/type/definition.js +++ b/src/type/definition.js @@ -92,9 +92,9 @@ export function assertInputType( ): GraphQLInputType { invariant(isInputType(type), validationError( - `Expected ${String(type)} to be a GraphQL input type.`, + source, typeNode, - source)); + `Expected ${String(type)} to be a GraphQL input type.`)); return type; } @@ -136,9 +136,9 @@ export function assertOutputType( ): GraphQLOutputType { invariant(isOutputType(type), validationError( - `Expected ${String(type)} to be a GraphQL output type.`, + source, typeNode, - source)); + `Expected ${String(type)} to be a GraphQL output type.`)); return type; } diff --git a/src/utilities/buildASTSchema.js b/src/utilities/buildASTSchema.js index c4fcaf8c96..4a731176b9 100644 --- a/src/utilities/buildASTSchema.js +++ b/src/utilities/buildASTSchema.js @@ -317,14 +317,14 @@ export function buildASTSchema( function produceObjectType(typeNode: TypeNode): GraphQLObjectType { const type = produceType(typeNode); invariant(type instanceof GraphQLObjectType, - validationError('Expected Object type', typeNode, source)); + validationError(source, typeNode, 'Expected Object type')); return type; } function produceInterfaceType(typeNode: TypeNode): GraphQLInterfaceType { const type = produceType(typeNode); invariant(type instanceof GraphQLInterfaceType, - validationError('Expected Interface type', typeNode, source)); + validationError(source, typeNode, 'Expected Interface type')); return type; }