Don't swallow errors from Engine#705
Merged
trevor-scheer merged 2 commits intomasterfrom Nov 14, 2018
Merged
Conversation
Throw errors we receive from Engine so we can provide the right feedback to users and handle the errors.
abernix
reviewed
Nov 15, 2018
| // throw new Error(errors); | ||
| // } | ||
| if (errors) { | ||
| throw new Error(errors.map(error => error.message).join("\n")); |
Member
There was a problem hiding this comment.
Can we DRY this up a bit by creating a helper? This seems like it's going to be a very common pattern that's going to perpetuate if we don't nip it in the bud right now.
At the very least, this might be a mapGraphQLErrorsToString(errors) (and be errors.map(error => error.message).join("\n")), but I'd even go so far to say we could have a assertNoGraphQLErrors(errors); function which would be all three lines here?
Contributor
Author
There was a problem hiding this comment.
Totally not opposed, and had considered this.
@jbaxleyiii mentioned to me that we're going to overhaul error handling, I'll leave it up to him if this is something I should take care of or not if this goes away soon anyway.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Throw errors we receive from Engine so we can provide the right feedback to users and handle the errors.
Tested for
client:checkandclient:pushusing invalid operations against a published schema.