Added option to show original locations and path in error #1
Conversation
|
@scf4 thanks! Can you update tests? |
|
Also, this might work better via an options hash versus two separate booleans. |
|
Yeah that's a better idea, check the new commit. Regarding tests I guess we'd need to use a real Apollo error object to test it properly. I don't have the time at the moment. Edit: I'm not familiar with CircleCI so unsure why it's failing? |
|
Looks like it's failing because |
|
Should be good now :) |
|
Can you update the test to make sure that the error retains the location and path after being passed through formatError? Also, can you update the docs? Thanks for the work on this @scf4. |
| ])); | ||
|
|
||
| this._name = name; | ||
| this._humanized_message = message || ''; | ||
| this._time_thrown = t; | ||
| this._data = d; | ||
| this._locations = (opts.showLocations && arguments[2] && arguments[2].locations) |
There was a problem hiding this comment.
Why are you using arguments here rather than opts?
There was a problem hiding this comment.
It's a truthy check of the opts. The line was initially below const t = (arguments[2] && arguments[2]... and I was keeping it brief, but I'll make the code a little clearer tomorrow!
The arguments come from the originalError in formatError.
| options: { | ||
| showLocations: true, | ||
| showPath: false, | ||
| }, |
There was a problem hiding this comment.
Please test formatError to make sure things are serializing properly
|
The error comes from the originalError object which is passed to formatError. What wouldn't be serialized properly? My use case is preserving the path info so I know which query caused which error when running multiple. I don't personally need locations but it could be relevant to some! |
|
It "should", but we want to add tests to make sure that if we create an error with such options, the options are preserved when the error is serialized out to the client |
|
Will merge after tests are adding. Wanna be super safe. Also, still wondering the use case here. Wouldn't I need to manually pass the location and path into the error data payload? If that's the case, are the booleans even necessary? |
|
@scf4 bump :) |
|
@thebigredgeek Apollo server adds the path and locations to errors by default. |
|
It would need to know to add them to the data object in the error constructor though, wouldn't it? |
|
formatError has originalError.path and originalError.locations. It passes them to the new custom error, which checks its opts (added in createError) to see if they should be returned in the object from serialize(). They don't get serialized into the |
|
Ahh I see now. Clever. Nice! Will merge in just a few |
|
Awesome thanks! |
No description provided.