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

Closed
cgetzen opened this issue Oct 13, 2021 · 3 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@cgetzen
Copy link
Contributor

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

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

@toothrot toothrot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 15, 2021
@toothrot toothrot added this to the Backlog milestone Oct 15, 2021
@AlexanderYastrebov
Copy link
Contributor

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

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
@golang golang locked and limited conversation to collaborators Nov 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
4 participants