Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net/http: TimeoutHandler should distinguish between timeout and requests closed by clients. #48948

Open
cgetzen opened this issue Oct 13, 2021 · 3 comments · May be fixed by #48993 or #49119
Open

net/http: TimeoutHandler should distinguish between timeout and requests closed by clients. #48948

cgetzen opened this issue Oct 13, 2021 · 3 comments · May be fixed by #48993 or #49119

Comments

@cgetzen
Copy link

@cgetzen cgetzen commented Oct 13, 2021

The new Handler calls h.ServeHTTP to handle each request, but if a call runs for longer than its time limit, the handler responds with a 503 Service Unavailable error and the given message in its body. (If msg is empty, a suitable default message will be sent.) After such a timeout, writes by h to its ResponseWriter will return ErrHandlerTimeout.
-- https://pkg.go.dev/net/http#TimeoutHandler

For incoming server requests, the context is canceled when the client's connection closes, the request is canceled (with HTTP/2), or when the ServeHTTP method returns.
-- https://pkg.go.dev/net/http#Request.Context

If a call runs for less than its time limit, but the client cancels the request, TimeoutHandler will respond the same way -- with an ErrHandlerTimeout.

This makes it hard to tell if the code is performing poorly and hitting a timeout, or if the clients are ending requests.

I propose a conditional is added here, to check if the context's error is a context.DeadlineExceeded or context.Canceled, and return different errors.

@gopherbot
Copy link

@gopherbot gopherbot commented Oct 14, 2021

Change https://golang.org/cl/356009 mentions this issue: net/http: distinguish between timeouts and client hangups in TimeoutHandler

@toothrot toothrot added this to the Backlog milestone Oct 15, 2021
@AlexanderYastrebov
Copy link
Contributor

@AlexanderYastrebov AlexanderYastrebov commented Oct 22, 2021

Related #22821

TimeoutHandler will respond the same way -- with an ErrHandlerTimeout.
This makes it hard to tell if the code is performing poorly and hitting a timeout, or if the clients are ending requests.

If client closed the connection it will not see error response. How would user code distinguish timeout and closed connection cases?

@gopherbot
Copy link

@gopherbot gopherbot commented Oct 22, 2021

Change https://golang.org/cl/357918 mentions this issue: net/http: enable TimeoutHandler custom error response

AlexanderYastrebov added a commit to AlexanderYastrebov/go that referenced this issue Oct 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants