eth: don't cancel ws header subscription context #2091
Conversation
| addEndpoint := func(endpoint string) error { | ||
| // Give ourselves a limited time to resolve a connection. | ||
| ctx, cancel := context.WithTimeout(ctx, defaultRequestTimeout) | ||
| timedCtx, cancel := context.WithTimeout(ctx, defaultRequestTimeout) |
There was a problem hiding this comment.
Good catch. I can see the problem was go p.subscribeHeaders(ctx, sub, h, log) below, and causing it to kill that subscription maintenance goroutine way too early.
We might run into trouble into the future if we assume the context to connectProviders just limits the time to connect, which is often the case with such functions in the standard library e.g. DialContext, but it also kills the goroutine. I think we should document the context parameter on (*multiRPCClient).connect and connectProviders, so that callers aren't tempted to put their own timeout on it, or anywhere higher up.
Use via (*ETHWallet).Connect => (*multiRPCClient).connect seems fine though because our Connector pattern is such that it shuts down the long running process rather than the initial connect attempt.
3 participants
Resolves #2087