-
Notifications
You must be signed in to change notification settings - Fork 10
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
Add possibility to reformat error in errorHandler #10
Comments
It depends what you are trying to do. You can either try to throw errors the way you need at the original place the error is thrown, try to customize errors before they get to the Here is an example of the latter: // …
async (ctx, next) => {
await next();
if (ctx.response.body?.errors)
ctx.response.body.errors.forEach((error) => {
// Add an extension…
if (!error.extensions) error.extensions = {};
error.extensions.foo = 'foo';
// Reword messages…
switch (error.message) {
case 'Internal Server Error':
error.message = 'Something is wrong on our end. Sorry!';
}
});
},
errorHandler(),
// … The upside to this is you can let the The downside is that while iterating the errors, you don't have the original error instances to work with - only what is intended to be exposed to the client. An idea if you need to customize the errors before they pass through the // …
errorHandler(),
async (ctx, next) => {
try {
await next();
} catch (error) {
// Customize the error…
// Then throw it again.
throw error;
}
},
// … A few potential gotchas with that:
Regarding your first use case for a format error API:
You could probably get away with the switch example I shared above; but if you needed to internationalise dynamic error message coming from resolvers that were deliberately exposed to the client then another approach would be better. You have to deliberately mark the error for exposure to the client in resolvers anyway, so at that time you could be creating the translated message. No custom Koa middleware needed. Regarding your second use case:
Those sorts of errors IMO are best customized at the point of failure. I do things like this in my resolver code: try {
response = await fetchTimeout(
`https://blockchain.com/tobtc?currency=${currencyCode}&value=1`
);
} catch ({ message }) {
throw new Error(
`Failed to fetch Bitcoin price for currency code “${currencyCode}” using Blockchain: ${message}`
);
} Note that I'm making a nice message for my API error log, but deliberately these sorts of resolver errors should not be exposed to the client — the I could see how maybe a function formatError(errorOriginal, errorExpose) {
// Return the object to be JSON stringified in the response body `errors` array.
// `errorExpose` is what would have been exposed by default.
return errorExpose
} Given the above discussion, do you really need it? |
Given 3 solutions, numbered, here is what I have: In my project I tried to use 1️⃣ , used custom errors with However, there are other errors, which are thrown not by me and which don't use my custom errors:
They don't have From proposed solutions: 1️⃣ - can't use, don't control them The winner is 2️⃣ , middleware before
Default message from source is hardcoded From proposed solutions: 1️⃣ - can't use, don't control it The winner here is 3️⃣ , middleware after So from this 2 cases the result is something like this: .formatErrorsAfterErrorHandler()
.errorHandler()
.formatErrorsBeforeErrorHandler() To me it looks unbelievably complicated. A single error middleware should handle all of this - catch errors, format them and return to the client.
All issues I had previously (#4, #8) are actually about formatting errors. I've tried to move from apollo-server monster to graphql-api-koa at least 2 times already and this error thing is always on my way. It gives me nightmares, I hate it to the point I drop moving attempts. I even wrote and asked about this in Twitter about a year ago. I used Here I have to dance with exposing errors using I don't want to write and use 3 middlewares and try to avoid rewriting So the answer is yes, I really need this more than anything in graphql server world 🙏 |
I'm not following why you can't control your database errors in resolvers - can't you wrap your query code in a try catch? If the queries are happening via abstractions like data loaders, you can still update the abstraction to throw an appropriate error. |
I can use try catch for database error in resolvers but don't do this because I'll need to use this wrapper in every resolver. I can avoid this by just throwing errors in resolvers and handle them later in centralized graphql error handler. |
You can make repetitive DB query error handling code DRY with helper functions. The best place to get the error message right is at the location of the error. You will have clean, sensible errors right from the source that way instead of cryptic DB errors flying out of your resolvers. It's good to self contain things too, instead of having the final errors determined elsewhere in your project via configuration. Part of the |
Even with this DRY helper functions I will still need to use them in every graphql resolver. 😞 I've tried to follow I love and prefer things simplicity and was taught by apollo to handle how errors are finally formatted and returned to client outside resolvers and it felt simple and natural. I didn't need any wrappers, helper functions, extra middlewares, I want this simplicity in 🙏 |
This also doesn't handle case like above where I need to set custom default error message instead of You may even want to customize error produced by All this cases can be handled by using just a plain function which executes lastly, has access to original error and returns final error which client will see. |
@jaydenseric any update to this? I kinda need this in my app badly 😢 |
I thought a fair bit about customising the language of the
I've published several GraphQL server and client related packages for which Apollo has a direct alternative, and almost everytime ex-Apollo users mistake different for "wrong", and push for API changes to be more like what is familiar to them. Similarity to Apollo APIs is not a design goal for my packages. It would be easy to say "yes!" to everyone and bend the API to grow users, but I'm not motivated by popularity. Everything I build is primarily motivated by technical excellence; I use these packages in my own projects. For example, Apollo users coming to Probably the biggest Apollo-ism is their push to get people to define their schema on the backend using GraphQL SDL strings, instead of composing it using the regular GraphQL.js API. Don't get me started on that! All my APIs use databases, and I have never had a problem with the way error messages currently work. I can see that there may be edge-cases beyond how I use |
I want to elaborate a bit on simplicity, at the risk of sounding argumentative. In every measurable way Apollo Server is not "simpler".
Here are all the exports of [
'GraphQLUpload',
'GraphQLExtension',
'gql',
'ApolloError',
'toApolloError',
'SyntaxError',
'ValidationError',
'AuthenticationError',
'ForbiddenError',
'UserInputError',
'defaultPlaygroundOptions',
'makeExecutableSchema',
'addCatchUndefinedToSchema',
'addErrorLoggingToSchema',
'addResolveFunctionsToSchema',
'addSchemaLevelResolveFunction',
'assertResolveFunctionsPresent',
'attachDirectiveResolvers',
'attachConnectorsToContext',
'buildSchemaFromTypeDefinitions',
'chainResolvers',
'checkForResolveTypeResolver',
'concatenateTypeDefs',
'decorateWithLogger',
'extendResolversFromInterfaces',
'extractExtensionDefinitions',
'forEachField',
'SchemaError',
'mockServer',
'addMockFunctionsToSchema',
'MockList',
'makeRemoteExecutableSchema',
'defaultCreateRemoteResolver',
'introspectSchema',
'mergeSchemas',
'delegateToSchema',
'defaultMergedResolver',
'transformSchema',
'AddArgumentsAsVariables',
'CheckResultAndHandleErrors',
'ReplaceFieldWithFragment',
'AddTypenameToAbstract',
'FilterToSchema',
'RenameTypes',
'FilterTypes',
'TransformRootFields',
'RenameRootFields',
'FilterRootFields',
'ExpandAbstractTypes',
'ExtractField',
'WrapQuery',
'SchemaDirectiveVisitor',
'PubSubEngine',
'PubSub',
'withFilter',
'ApolloServer'
] At least 7 jump out as related to errors alone. APIs that need to be documented, learned and maintained. |
I disagree with you on internalization. An app can be in language different from English and it's quite common to return errors in language of the app, frontend developers can then use this messages to show to end user. I understand what you write about Apollo, I've been using Apollo packages for a long time (and their Meteor framework before too), know how many bugs are hidden behind their simplicity and personally won't use or suggest any of their packages. Even though there are only two exports, Apps have different requirements and unfortunately many of them can't be structured and used same way you do. The feature I ask for covers so many edge cases, Apollo is not the only one who has it - I checked a few other packages and all of them have it: koa-graphql, express-graphql, hapi-graphql. I absolutely love your packages and enjoy using them but here I strongly believe this is not an added unnecessary complexity, it is really useful and needed for so many things. |
A small update: I've being using It becomes kinda custom @jaydenseric first, huge thank you for the library man, really appreciate the simplicity and absolutely love it ❤️ I've re-read carefully your proposed solutions today and I'm wondering if your opinion on |
Thanks for the update; glad to see you've been finding it useful. Your feedback based on solid experience is appreciated :)
I've been working full time for over a year now doing R&D to pivot from Node.js, centralised npm and bloated packages designed around build steps to Deno, decentralised CDN's and standard import maps, buildless, with type safety via TypeScript JSDoc comments. I had to first figure out the best way to make universal ESM packages with type safety that work for both Node.js and Deno, then had to one-by-one republish all of my dev tools, and then use those updated dev tools to republish all of the raw ingredients needed to work on a from-scratch web application framework for Deno ( Now I am updating all of the web apps I maintain to switch from Node.js and Next.js deployed with Vercel to Deno and Ruck, deployed with Fly.io. Once that is done, I can turn my attention back to GraphQL APIs because I want to be able to build them using Deno as well. I haven't actually had the chance to work much in GraphQL API backends in the last 2 years due to all this R&D work, and my last contract before this was working on an app that was powered by an Elixir GraphQL backend so I couldn't work directly on it or use any of my JS GraphQL backend stuff. The GraphQL APIs for my own apps have been pretty reliable and didn't need a lot of maintenance. I'm not 100% sure what the dream GraphQL API backend in Deno would look like; could it be universal for both Node.js and Deno or will that be too difficult, or compromise ergonomics? It probably doesn't make sense to work with Koa anymore. Hopefully this work won't be blocked by GraphQL.js not providing real ESM or using Node.js API's which is problematic for Deno. I was hoping that by the time I was working on the backend again GraphQL.js would have improved their ESM game. A year working pro-bono on open source has smashed my savings so I have to take up contract work for money very soon, so it might have to be after that that I can immerse in the Deno GraphQL API problem space. |
It would be nice to be able to reformat error in
errorHandler
. It's useful when you need to modify error before sending to client, for example:Internal server error
message to other languageThe api might be as simple as:
I keep hitting this actually simple cases where I just want to format my errors before sending them to client and current
errorHandler
is too restrictive in what I can do. However, I'm trying to avoid writing custom middleware instead oferrorHandler
and struggling to find solutions.@jaydenseric help 🙏
The text was updated successfully, but these errors were encountered: