Rethink ApolloError #6053
Comments
|
Another implementation of |
Not sure if we'll stick with this: depends on what we end up doing in #6053. Maybe it will still be helpful for libraries to be able to use AS errors without depending on a specific pinned version of AS. But for now this is simpler.
|
On That said, it's possible we might decide that errors do deserve their own package, so that a library could implement an ApolloError (if it still exists) or throw a specific error like BadInputError without needing a dependency on |
|
Worth looking at #6316 to see if some of its ideas are relevant. |
|
Major progress on version-4 in #6355. We still need to describe those changes and maybe make a few more like renaming formatApolloErrors. |
Previously, errors that weren't specially cased as part of the "request pipeline" were not observable by plugins. (In some but not all cases, they were visible to formatError hooks.) Some "HTTP-level" errors were handled by returning text/plain responses, whereas many other errors were handled by returning GraphQL-style JSON responses (using the formatError hook). This change makes things more consistent by returning all errors as JSON responses (using the formatError hook). In addition, for better observability, several new top-level plugin APIs are introduced that are called when various kinds of errors occur. Specifically: - New plugin hook `startupDidFail`. This is called once if startup fails (eg, if a serverWillStart plugin throws or if gateway fails to load the schema). (If this hook throws, its error is logged.) Future calls to executeHTTPGraphQLRequest (if the server was started in the background) now return a formatted JSON error instead of throwing. - New plugin hook `contextCreationDidFail`. (If this hook throws, its error is logged.) As before, these errors are sent as formatted JSON errors. - New plugin hook `invalidRequestWasReceived`. This is called for all the errors that occur before being able to construct a GraphQLRequest from an HTTPGraphQLRequest; this includes things like the top-level fields being of the wrong type, missing body, CSRF failure, etc). These errors are now sent as formatted JSON instead of as text/plain. - New plugin hook `unexpectedErrorProcessingRequest`. This is called if `processGraphQLRequest` throws (typically because a plugin hook threw). We are making the assumption that this means there is a programming error, so we are making the behavior change that the thrown error will be logged and "Internal server error" will be sent in the HTTP response. - Fix grammar of "GET supports only query operation" and don't return it if the operation is unknown. Fixes #6140. Part of #6053.
Previously, errors that weren't specially cased as part of the "request pipeline" were not observable by plugins. (In some but not all cases, they were visible to formatError hooks.) Some "HTTP-level" errors were handled by returning text/plain responses, whereas many other errors were handled by returning GraphQL-style JSON responses (using the formatError hook). This change makes things more consistent by returning all errors as JSON responses (using the formatError hook). In addition, for better observability, several new top-level plugin APIs are introduced that are called when various kinds of errors occur. Specifically: - New plugin hook `startupDidFail`. This is called once if startup fails (eg, if a serverWillStart plugin throws or if gateway fails to load the schema). (If this hook throws, its error is logged.) Future calls to executeHTTPGraphQLRequest (if the server was started in the background) now return a formatted JSON error instead of throwing. - New plugin hook `contextCreationDidFail`. (If this hook throws, its error is logged.) As before, these errors are sent as formatted JSON errors. - New plugin hook `invalidRequestWasReceived`. This is called for all the errors that occur before being able to construct a GraphQLRequest from an HTTPGraphQLRequest; this includes things like the top-level fields being of the wrong type, missing body, CSRF failure, etc). These errors are now sent as formatted JSON instead of as text/plain. - New plugin hook `unexpectedErrorProcessingRequest`. This is called if `processGraphQLRequest` throws (typically because a plugin hook threw). We are making the assumption that this means there is a programming error, so we are making the behavior change that the thrown error will be logged and "Internal server error" will be sent in the HTTP response. - Fix grammar of "GET supports only query operation" and don't return it if the operation is unknown. Fixes #6140. Part of #6053.
|
In addition to writing the docs, @trevor-scheer and I have some questions we're thinking about with regards to the exported XError types. It's now quite easy to define your own subclasses with particular error codes. So the questions are:
|
@glasser I agree we should remove them.
No, I think exposing enum with codes would be enough.
No, I think they don't add a lot of value.
This function feels more like ES5 (before modern ES6 class syntax). export class PersistedQueryNotSupportedError extends GraphQLErrorWithCode {
constructor() {
super(
'PersistedQueryNotSupported',
ApolloServerErrorCode.PERSISTED_QUERY_NOT_SUPPORTED,
);
this.name = 'PersistedQueryNotSupportedError';
}
}But I don't think that's a big saving compared to messing with class prototypes dynamically. I implement above suggestions in #6705 |
ApolloError in apollo-server-errors adds some functionality to GraphQLError from graphql-js. graphql-js may have already caught up with this to some degree, and other functionality perhaps is general-purpose enough that it could easily just move to graphql-js. Let's figure out exactly what the value of having our own subclass is and see what we can do to reduce complexity.
The text was updated successfully, but these errors were encountered: