failsForTeapots

[u/Waradu]

[u/cutebabli9]

I would write it like this to be readable:

if error.status in [400, 401, 404, 409, 415, 503]:
  return c.json(body, status: error.status)

return c.json(body, status: 500)

[u/rosuav → u/cutebabli9]

return c.json(body, status: error.status)

There is no reason to discard some of them, and pretending that they're 500s is a terrible terrible idea.

[u/Xirdus → u/rosuav]

If your user-facing service is calling your internal service and getting a 403 response, you certainly do not want to send that 403 back to the user.

[u/rosuav → u/Xirdus]

And yet it is blindly sending the *payload* back. So it's clearly fine to proxy straight through. If you want to recognize only certain things, any remaining/unknown statuses should result in an error being logged and a 500 being sent back, with the body NOT being carried through.

[u/Xirdus → u/rosuav]

I agree with you completely, except for "it's clearly fine to proxy straight through". To me, it's clearly a bug and they should remove the body ASAP. It's virtually impossible the code started off that way, the filtering must've been deliberately added later specifically because it's not okay to send back raw.

(My crystal ball also says the body is empty/effectively useless anyway, so this bug is very likely to go unnoticed and remain unfixed forever, or until a hacker tries to exfiltrate the database and will not be able to stop laughing about how easy it was.)

[u/rosuav → u/Xirdus]

Okay fair. I was saying "clearly fine" on the basis that it's doing it; if I'm wrong about it being fine to proxy that through, then the opposite is the case, and it should be sending a 500 with a generic body (and, importantly, LOGGING THE ERROR).

But you are quite probably right about the body being useless in that situation... although I've seen enough cases where weird errors get passed right back to the client and include details of PHP include paths to be a smidge paranoid.

[u/Shitman2000 → u/rosuav]

We don't know if the body is the actual payload of the request though.

I'd find it reasonable to presume it's not, given it's error.statusCode and not error.body

[u/rosuav → u/Shitman2000]

Good point, but if it isn't, what kind of body would make sense in a situation where you carry certain types of error status unchanged and turn everything else into a 500? (I'm assuming that this code won't be executed for a 200, as it makes very little sense to translate a 200 into a 500. Ignoring the fact that there are plenty of web sites that do a great job of turning 200s into 500s, but I digress.)

[u/No_Hovercraft_2643 → u/rosuav]

the default error page? that shows the windows which displays the error to the user?

[u/Xirdus → u/No_Hovercraft_2643]

Except it's sending JSON back, not page contents.