Graphql-js: Design proper error names

Created on 3 May 2019  路  6Comments  路  Source: graphql/graphql-js

GraphQL-JS throws a variety of errors in normal everyday use e.g. GraphQLErrors, TypeErrors, and even regular JavaScript Error objects. All of these can occur because the client supplies garbage values to the server.

However, on the server side, I want to make a distinction between internal server errors (e.g. assertions, which should crash the server) and client errors (which should return a good client error message to the client).

To accomplish this, I tried to write a good formatError() function to give to express-graphql.

I cannot get this right, because e.g. graphql throws TypeErrors, which are indistinguishable from a server-side programming bug that causes a NodeJS TypeError.

So the proposal would be to ensure that graphql client errors are distinguishable from server errors, e.g. by giving all errors proper values for their 'name' and 'code' properties. This is also the direction in which NodeJS is going at the moment, see https://nodejs.org/api/errors.html#errors_error_code

enhancement

Most helpful comment

Has there been any progress on this item lately? I have the same use-case as @rogierschouten - I'm formatting errors and trying to decide what is acceptable to expose to the client, and what should be masked. I use Apollo, and it seems that all errors from Apollo are wrapped by ApolloError, which makes it easy to filter - but the GraphQL errors seem to be all over the place. For now I'm doing a combination of checking if the error is a GraphQL error, and doing some text matching on the error message. Definitely not a great solution. Is there anything I can do to help out here?

All 6 comments

@IvanGoncharov is more exploration needed than simply replacing all throw Foo by a proper throw GraphQLError? What is the bottleneck, is there any thinking I can help you with?

@rogierschouten Sorry I didn't read carefully enough during initial triage.
I saw you mentioned name and code property and assumed it was the main topic of this issue. So we definitely want to have some mechanism for distinguishing different types of errors (parsing, validation, execution, etc.) and we discussed it on GraphQL WG and current consensus is that we need to add it in the GraphQL specification first and also expose them in the formatError.

About the main topic of this issue on using a mixture of Error subclasses, it is definitely something that we should address and it doesn't require any spec changes.

is more exploration needed than simply replacing all throw Foo by a proper throw GraphQLError?

I still think we need to distinguish between development errors, graphql-js own invariants, and errors triggered by clients. So I need to do some research on what other big libraries that extensively use callbacks are using e.g. React.

Currently, I'm trying to finish two high priority issues for 14.5.0 release but this issue will be next in my queue so will try to resolve it next week.

I still think we need to distinguish between development errors, graphql-js own invariants, and errors triggered by clients.

I agree, the most important thing is being able to distinguish between the different classes of errors. Thx!

we discussed it on GraphQL WG and current consensus is that we need to add it in the GraphQL specification first and also expose them in the formatError.

I am also encountering the same problem and would love to see the differentiation you are describing.
@IvanGoncharov Is there an Open Issue on the graphql-spec page ?

@hassenc I'm not sure, but it should be something inside notes of previous meetings:
https://github.com/graphql/graphql-wg/tree/master/notes

Has there been any progress on this item lately? I have the same use-case as @rogierschouten - I'm formatting errors and trying to decide what is acceptable to expose to the client, and what should be masked. I use Apollo, and it seems that all errors from Apollo are wrapped by ApolloError, which makes it easy to filter - but the GraphQL errors seem to be all over the place. For now I'm doing a combination of checking if the error is a GraphQL error, and doing some text matching on the error message. Definitely not a great solution. Is there anything I can do to help out here?

Was this page helpful?
0 / 5 - 0 ratings