Log a status of 499 if the client cancels the request #68
Conversation
Previously a client-cancelled request was logged as a proxy error, which is misleading. The other situation in which requests may be cancelled is where we're forced to terminate incomplete requests during draining. In this case, we'll continue to respond with a 502, but add additional context to the logs for clarity.
If we have to cancel a request because we're draining the target and the upstream hasn't sent us a response yet, then 504 is arguably a more appropriate status. The upstream didn't error, it just wasn't fast enough given the situation.
| @@ -15,6 +15,10 @@ import ( | |||
| "time" | |||
| ) | |||
|
|
|||
| const ( | |||
| HTTP_STATUS_CLIENT_DISCONNECTED = 499 | |||
There was a problem hiding this comment.
A small nit, and I appreciate it's too late for a review since the changes landed already.
Outside of the Go standard library, where all-caps constants are often used for historic or compatibility reasons (e.g., POSIX permissions constant, syscall flags, etc.), HTTP_STATUS_CLIENT_DISCONNECTED could be made more Go-ish as StatusClientClosedRequest (or even starting with lower-case to avoid exporting the symbol if there is no need to reuse it elsewhere).
Eventually, some linters or other Go developer might pick on this, too. 😄 Per the:
Why the name change of the variable? To keep it more aligned with what Nginx initially called it. This might have become a canon now since others (i.e., Cloudflare) picked it, too. This is also a custom status code introduced by Nginx; it does not exist (to best of my knowledge) in the original RFC, etc.
There was a problem hiding this comment.
D'oh, thanks @kwilczynski! Despite writing Go for years I still accidentally write an all-caps constant from time to time. Old habits, and all that :) Thanks for pointing it out.
Good call on the name change too. I'll take care of it.
Previously a client-cancelled request was logged as a proxy error, which is misleading. We can instead log it as 499, which is unambiguous and matches a convention used elsewhere.
Another situation in which requests may be cancelled is where we're forced to terminate incomplete requests during draining, because the drain timeout has expired before they've completed. In that case, we'll respond with a 504 Gateway Timeout, and log some additional context for clarity.
Closes #59