Skip to content
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

Return informative errors from GraphQL endpoint #1932

Merged
merged 3 commits into from
Apr 28, 2021

Conversation

lperson
Copy link
Collaborator

@lperson lperson commented Feb 22, 2021

Description

I noticed while working on a web component that all errors coming back from the graphql endpoint were empty objects. When an error was thrown in a query or mutation, this code would be called with graphQLErrors equal to an array consisting of an empty object -- [{}]. The error thrown in the client would be simply error Error: GraphQL error: undefined which doesn't let us give the user any useful information.

This PR has two commits. The first commit could be a solution on its own depending on what we think should be disclosed in the web app.

The first commit improves the situation a little, resulting in an error with this message: error Error: GraphQL error: Internal server error.

The second commit returns this (as an example) error Error: GraphQL error: Invalid Twilio credentials.

Also, I believe this is dead code now.

Checklist:

  • I have manually tested my changes on desktop and mobile
  • The test suite passes locally with my changes
  • If my change is a UI change, I have attached a screenshot to the description section of this pull request
  • My change is 300 lines of code or less, or has a documented reason in the description why it’s longer
  • I have made any necessary changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • My PR is labeled [WIP] if it is in progress

@lperson lperson changed the title Return informative errors from GraphQL endpoint:w Return informative errors from GraphQL endpoint Feb 22, 2021
Copy link
Collaborator

@schuyler1d schuyler1d left a comment

Choose a reason for hiding this comment

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

The point of the logic there is to restrict what kind of error is revealed. If you want to make changes, then do so up here:
https://github.com/MoveOnOrg/Spoke/pull/1932/files#diff-846e308afe343fa0238d38943beed033bec2ab41582b904029a4ef0106d634a1R194-R196
and enable SHOW_SERVER_ERROR=1 or DEBUG=1 on your instance

@lperson
Copy link
Collaborator Author

lperson commented Feb 23, 2021

@schuyler1d
Copy link
Collaborator

Yes!

Copy link
Collaborator

@schuyler1d schuyler1d left a comment

Choose a reason for hiding this comment

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

👍

@schuyler1d schuyler1d added the S-ready for stage-main (qa) Status (ADMINS ONLY): PR label for those ready to be added for stage: Approved, tests, etc label Mar 8, 2021
@schuyler1d schuyler1d merged commit d70a8c9 into StateVoicesNational:main Apr 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-ready for stage-main (qa) Status (ADMINS ONLY): PR label for those ready to be added for stage: Approved, tests, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants