Skip to content

Errors: fix how error messages represent arrays#1333

Merged
mjmahone merged 2 commits intographql:masterfrom
IvanGoncharov:wrapPrint
Jun 1, 2018
Merged

Errors: fix how error messages represent arrays#1333
mjmahone merged 2 commits intographql:masterfrom
IvanGoncharov:wrapPrint

Conversation

@IvanGoncharov
Copy link
Copy Markdown
Member

@IvanGoncharov IvanGoncharov commented May 2, 2018

I noticed this bug when I worked on #1332 I accidentally returned an array of types from typeResolver function and got this error:

Abstract type FooInterface must resolve to an Object type at runtime for field
Query.foo with value \"[object Object]\", received \"FooObject\". Either the 
FooInterface type should provide a \"resolveType\" function or each possible types 
should provide an \"isTypeOf\" function.

The relevant part here is:

must resolve to an Object type at runtime ..., received \"FooObject\".

It required me some time in the debugger to figure out that I wrapped GraphQLObjectType into an array. But it's impossible to figure out this from error message itself:

`value "${String(result)}", received "${String(runtimeType)}". ` +

String converts arrays into a comma-separated list of values so if array consists of one element it just converts that element to string. So instead of [FooObject] or something similar I got FooObject insider error.

Since calling validate and validateSchema are optional almost any error can be affected by this problem. So I think we need a library-wide solution to this problems.
The ideal solution would be to use utils.inspect instead of String but it is Node-specific and couldn't be used in browsers.

That's why I created jsutils/inspect.js:

export default function inspect(value: mixed): string {
  if (Array.isArray(value)) {
    return '[' + String(value) + ']';
  }
  return String(value);
}

Copy link
Copy Markdown
Contributor

@mjmahone mjmahone left a comment

Choose a reason for hiding this comment

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

This is great! Thanks for making array-errors more readable throughout the package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants