Fix the response status code logged in 'query stats' on error #8407
Conversation
Signed-off-by: Marco Pracucci <marco@pracucci.com>
pkg/frontend/transport/handler.go
Outdated
|
|
||
| // If we've been able to get the HTTP response from the error, then we send | ||
| // it with the right status code and response body content. | ||
| if res != nil { |
There was a problem hiding this comment.
Note to reviewers: the logic should have not been changed at all (at least not intentionally). What I'm doing here is exactly what we did before calling httpgrpc.WriteResponse(w, resp) and httpgrpc.WriteError(w, err).
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
pkg/frontend/transport/handler.go
Outdated
| httpgrpc.WriteError(w, err) | ||
| // Otherwise, we do fallback to a 5xx error, returning the non-formatted error | ||
| // message in the response body. | ||
| statusCode = http.StatusInternalServerError |
There was a problem hiding this comment.
nit: I think it works better as a fallback if you put this line at the top of the function, then any time we cannot or miss updating it, the result will be http.StatusInternalServerError
There was a problem hiding this comment.
I see the problem but I'm not convinced that having a default is the right thing to do. I want to make sure that we always return the right status code. To solve it, I took a different approach and I removed the named return parameter so that it's intentional what we return: 793436b
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
What this PR does
Today, if an error occurs in the query-frontend middlewares, the "query stats" log contains
status_code=0. However, we actually return a valid HTTP response to the client, with a legit status code, so we should log the actualstatus_code.This PR fixes it.
Which issue(s) this PR fixes or relates to
N/A
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX].about-versioning.mdupdated with experimental features.