Skip to content

Errors: fix how error messages represent arrays #1333

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 2 commits into from
Jun 1, 2018

Conversation

IvanGoncharov
Copy link
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
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.

@mjmahone mjmahone merged commit 324768a into graphql:master Jun 1, 2018
IvanGoncharov added a commit that referenced this pull request Jun 1, 2018
Working on #1333 I noticed that you can pass arrays as scalars in a query variable, e.g. [SWAPI example](http://graphql.org/swapi-graphql/?query=query%20(%24id%3A%20ID)%20%7B%0A%20%20person(personID%3A%20%24id)%20%7B%0A%20%20%20%20name%0A%20%20%7D%0A%7D&variables=%7B%0A%20%20%22id%22%3A%20%5B1%5D%0A%7D)
This PR is based on #925 but fixing same issue for types other than `String` e.g.
```js
Number([1]) // 1
```
@IvanGoncharov IvanGoncharov deleted the wrapPrint branch June 6, 2018 15:51
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