search

safe-global / safe-client-gateway

Serves as a bridge for the Safe{Wallet} clients (Android, iOS, Web)
https://docs.safe.global
MIT License
27 stars 64 forks source link

Return `502` for gateway validation errors #2111

Open iamacook opened 1 week ago

iamacook commented 1 week ago

Summary

When validating the response of a service, we return a 500. This is a vague response.

This changes server validation-related error codes to 502.

Changes

hectorgomezv commented 4 days ago

This error happened because the way AppModule was initialized in list-queued-transactions-by-safe.transactions.controller.spec.ts prevented the injection of the Interceptors/Filters, and therefore the ZodError filter wasn't applied. I've created and merged a PR to fix this in https://github.com/safe-global/safe-client-gateway/pull/2127. After that, rebasing this onto main will make the tests pass 🙂

Question: Why do we need to change the response status code to 502?

For validation errors, the appropriate status code is generally 400 Bad Request, as it indicates an issue with the request sent by the client. However, since this is an internal validation error (an issue occurring within our service logic rather than due to a client error), 500 Internal Server Error is more suitable. This code reflects that the error was caused by the server’s internal processing rather than by the client’s input.

I don't have a strong opinion regarding this, but I think distinguishing 500/generic errors from 502 errors caused by server-side validation can be beneficial 🙂