-
-
Notifications
You must be signed in to change notification settings - Fork 40
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
Default error handler #9
Comments
It appears that this was implemented for subscriptions without any possibility to override the default error handler with a customised one: re-graph/src/re_graph/internals.cljs Lines 148 to 161 in 78ac8b5
|
Hi @urzds The code you link to is specifically for "general" errors, not errors relating to specific queries. I don't know when these might happen, I've not seen them using lacinia-pedestal, but I don't think it's a common use case to be able to handle it (please correct me if I'm wrong, and if you are seeing this specific case - I'd like to know what the scenario is). If a query has an error the message from the server will be of type "data" which contains errors (see this issue). All messages are passed to your callback handler, it is up to the callback handler to check if there are errors in the response and do something appropriate. It was this use case I had in mind when I wrote this issue - that all handlers might check for errors and then handle it the same way, which would cause duplication. |
I am creating a
At least Lacinia 0.29.0 / Lacinia Pedestal 0.10.0 sends errors for subscriptions this way. (I could not quickly figure out how the GraphQL spec requires this case to be handled). Since re-graph does not pass on these errors, my application has no way of handling this itself and hence cannot display any kind of error message. Could of course also be a bug in Lacinia... |
It's a pity that we cannot specify the custom error handler for cases when the type is actually a known error - in cases like when the backend is throwing e.g. "credentials expired" errors. |
Rather than having possibly duplicated error handling logic in every response handler it might be useful to have a catch-all handler
The text was updated successfully, but these errors were encountered: