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

Error Handling? #115

Closed
pyrossh opened this issue Feb 24, 2016 · 4 comments
Closed

Error Handling? #115

pyrossh opened this issue Feb 24, 2016 · 4 comments

Comments

@pyrossh
Copy link
Contributor

pyrossh commented Feb 24, 2016

Graphql go right now swallows all errors and returns them back to the user. Sometimes we get database errors like SQL State: conflicting foreign key and this error goes back to the user. This is related to graphql/graphql-js#28.

We should add some add some ErrorHandling logic to prevent some of these errors for going back and also returning them back to the application so that it can know what to do with it (like log, mail).

The graphql scala https://github.com/sangria-graphql/sangria version have done a really good way of handling it here http://sangria-graphql.org/learn/ Maybe we should try their approach?

@bbuck
Copy link
Collaborator

bbuck commented Aug 9, 2016

Is this not already possible with the current state of the library? I'm not sure if this feature existed at the time this issue was posted but as it stands a resolve function now has a return type of (interface{}, error) letting you receive and process any errors you have.

Should this issue be closed?

@bsr203
Copy link

bsr203 commented Aug 9, 2016

I am bit worried about not much activities on pending pull requests and issues. @chris-ramon and @sogko poured ton of effort initially and wonder how they feel about the project now. May be it is a good idea to include @bbuck @pyros2097 and other frequent users of this library to have commit rights?

@pyrossh
Copy link
Contributor Author

pyrossh commented Aug 9, 2016

Actually even I've stopped using this library and gone back to using graphql-js and the other go-graphql library and am in the process of creating one for rust(just for fun).
Btw this issue should be closed since I made the pull request long ago and fixed the error handling issue.

@pyrossh pyrossh closed this as completed Aug 9, 2016
@zonr
Copy link
Contributor

zonr commented Jun 1, 2018

Storing gqlerrors.Error in graphql.Result.Errors will provide structured and more complete error context to the library users and therefore makes custom exception handing easier. graphql/graphql-js has made the transition in graphql/graphql-js#118. Should we follow their changes?

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

No branches or pull requests

5 participants