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

client timeouts vs deadline context #1398

Open
bhassink opened this issue Oct 27, 2022 · 0 comments
Open

client timeouts vs deadline context #1398

bhassink opened this issue Oct 27, 2022 · 0 comments

Comments

@bhassink
Copy link

bhassink commented Oct 27, 2022

There seems to be a documentation error in client.go...

// Timeout is a cumulative timeout for dial, write and read, defaults to 0 (disabled) - overrides DialTimeout, ReadTimeout,
// WriteTimeout when non-zero. Can be overridden with net.Dialer.Timeout (see Client.ExchangeWithDialer and
// Client.Dialer) or context.Context.Deadline (see ExchangeContext)

From the code, Client.Timeout only overrides Client.DialTimeout. There is also this documentation for Client.ExchangeContext()...

// ExchangeContext acts like Exchange, but honors the deadline on the provided
// context, if present. If there is both a context deadline and a configured
// timeout on the client, the earliest of the two takes effect.

It's awkward if the context deadline is larger than the default dnsTimeout constant of 2s. In that situation, all the client timeouts must be set to a value equal or larger than the context timeout in order to get the desired behavior.

IMO, implementation should be updated to have Client.Timeout behave as documented and supersede Client.DialTimeout + Client.ReadTimeout + Client.WriteTimeout. Additionally, a context deadline should supersede Client.Timeout (or Client.DialTimeout + Client.ReadTimeout + Client.WriteTimeout).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant