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

Introducing some error type to the spec #135

Closed
arunoda opened this issue Jan 4, 2016 · 7 comments
Closed

Introducing some error type to the spec #135

arunoda opened this issue Jan 4, 2016 · 7 comments
Labels
🗑 Rejected (RFC X) RFC Stage X (See CONTRIBUTING.md)

Comments

@arunoda
Copy link

arunoda commented Jan 4, 2016

Right now, when some error happens inside GraphQL schema or in the syntax wise we get errors as whole. Then it's our job to decide what to do with it.

But it would be great, if we can have a set of GraphQL error types(in the spec) which deals with different use cases.

  1. Syntax/Type Errors - These errors are for the errors in client provides query
  2. Runtime Errors - These are the error occurred in runtime specially with syntax errors and network related errors.
  3. User Errors - These are the errors we need to pass to the user.

Right now, in the graphql-js implementation, we get all these errors as one type. But, it'll be useful if we've some mechanism to differentiate them.

So, it would be great GraphQL spec to define these errors. So, the implementations can use them. Also, the people who write GraphQL schemas can use them to define User Errors.


Then, in the transport layers we can check for these three types of errors and do necessary actions.
Is there any plan to have these in spec. If so, I'd like to work on them.

@leebyron
Copy link
Collaborator

leebyron commented Jan 9, 2016

Yeah I think this is a good idea. Any concrete proposals on what this should actually look like in terms of the spec?

@arunoda
Copy link
Author

arunoda commented Jan 9, 2016

Awesome.
Here's one of our member @mnmtanish's writeup on this.

Here's our idea for the spec.

We've a type called GraphQLUserError, It's a custom error type. Schema creators can import and throw that if needed.

This will do nothing much special, but it will allow us to filter these errors from the rest in the transport level.

What do you think?

@leebyron
Copy link
Collaborator

leebyron commented Jan 9, 2016

One clarification here, as this topic was also brought up in #117. Your error type number 3 I interpreted as "user of the GraphQL endpoint" aka "the client developer" and not "user of the product". I think it is useful to disambiguate between errors that are due to a bad query vs errors that are due to something having gone wrong within the server (e.g. a database was down or some server code threw an error).

For errors that are the fault of the product end-users, we don't want to spec an Error type because different products and different interfaces where user-errors could occur will all represent different kinds of information and should be represented by data. While you're free to refer to this type as "Error" if you like, from GraphQL's point of view it is just another data type.

For example, an error when filling out a form might want to report back which part of the form had an issue so a UI could highlight it. Or maybe an error about not being able to deliver a message in an IM client might want to include information about if the user should retry or if the server is retrying on their behalf. This information is hard to globally standardize as one single type since each context has these specific details. So it's important to distinguish between the data that represents a "user error" vs the error that represents the GraphQL server's inability to accomplish what a GraphQL query requested.

@leebyron
Copy link
Collaborator

leebyron commented Jan 9, 2016

@arunoda it sounds like that idea could be easily accomplished without any changes to the spec. We could add some thrown error post-processing to determine if (or how) an error should be reported or not. That is something we were planning to add anyhow to improve error debugging in general

@arunoda
Copy link
Author

arunoda commented Jan 9, 2016

I get it. It's hard to formalize a common error type. What we mean by the [3] is, we should not send programming errors to the client side. But, anyway it's an application level decision, not something GraphQL should decide.

So, we like the graphql-js's implementation of sending all the errors and let the transport(app layer) to do whatever it needs.

Now, we need to find a way to differentiate these syntax errors and what the schema really need to send to the client.

For an example, if there is a credit card related issue. we don't wanna send the original message to the client. (This is based on the never trust the client principle).
So, instead schema will send a error of GraphQLUserError.

Why we need this in the schema (even some mentioning)

We may be using schemas written by different people. If everyone following the same then it's easy for app/transport layer to decide on errors.

But, I'm not sure where it should goes in the spec.

@leebenson
Copy link

I don't have anything useful to add, but wanted to give an example of how I'm using @arunoda and @mnmtanish's graphql-errors package pre-spec as a quick interim hack, in case there are others following the thread.

Example here.

@leebyron leebyron added the 🗑 Rejected (RFC X) RFC Stage X (See CONTRIBUTING.md) label Oct 2, 2018
@leebyron
Copy link
Collaborator

leebyron commented Oct 2, 2018

Moving this to rejected since it is aging.

Since discussing this issue, a common best practice has been to include user errors as part of the schema itself so they can contain domain specific information.

However I'd love to see a new proposal which adds error codes to differentiate syntax/validation/runtime errors, especially if it could make integration with graphql-cats like testing easier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🗑 Rejected (RFC X) RFC Stage X (See CONTRIBUTING.md)
Projects
None yet
Development

No branches or pull requests

3 participants