search

wundergraph / graphql-go-tools

GraphQL Router / API Gateway framework written in Golang, focussing on correctness, extensibility, and high-performance. Supports Federation v1 & v2, Subscriptions & more.
https://graphql-api-gateway.com
MIT License
685 stars 129 forks source link

fix: simplify error string #896

Closed StarpTech closed 1 week ago

StarpTech commented 1 week ago

We never used the extended version for error reporting. It become even an issue when the error has to be printed somewhere because the error message was too long. If someone wants to get the downstream error, he can do so by accessing them on the SubgraphError type.

flymedllva commented 1 week ago

@StarpTech @jensneuse

We are already used to this format and it seems that we have to change something to understand which path in which subgraph had an error :( Can we use cosmo modules to restore this format? In general in cosmo things change so often that we don't even have time to update and adapt to these changes. I would like to bring error to 1 view or as suggested here https://github.com/wundergraph/cosmo/issues/957

StarpTech commented 1 week ago

Hi @flymedllva, thanks for the feedback. Could you elaborate how you used it? The next iteration of the module system will cover it https://github.com/wundergraph/cosmo/pull/1063

flymedllva commented 1 week ago

We use a self-hosted router, without the other dependencies like keycloak, clickhouse, etc. Our subgraphs write errors not hidden from users, and we have 2 routers for different clients. The 1st client is backoffice, where we don't touch errors at all (the old format you removed) and we want to output the most complete error information there for quick problem solving, without using logs/tracing and so on. The 2nd client is frontend, where we have a proxy hiding errors (any error => “INTERNAL”) if the code of one of the subgraphs is 5**, and we write in our logs the full error (which was in MR) and quickly use our indexed logs and request id to find which service started responding unexpectedly. Now I don't understand how we can do this logic, because you removed this information from the response and it seems that now we need to deal with error transormation in router.

flymedllva commented 1 week ago

I reviewed the RFC, looks appropriate, wrote my questions there

Thank you, really looking forward to these changes!

StarpTech commented 1 week ago

Hi @flymedllva, how did you access those information before? How did you rely on it technically?

flymedllva commented 1 week ago

I honestly don't understand the question. Just when you returned an error + path + subgraph

And we were fine

I'll be fine if cosmo can change values in errors based on subgraph data and their responses, we'll come up with a convenient format that fits our general requirements and that's it

Thank you!