rpcclient: Use passed contexts over websocket connections. #2198
Conversation
JoeGruffins
force-pushed
the
wsctx
branch
3 times, most recently
from
d952fc6
to
f1067f5
Compare
|
Can write tests if needed. |
JoeGruffins
force-pushed
the
wsctx
branch
3 times, most recently
from
ce45637
to
0d483f5
Compare
|
This will also need a rebase over master as it conflicts with #2205. |
The passed context was ignored until a reconnect. This kills the client when the passed context is done. This is much more useful for consumers who need the client to shut down when their context if finished.
|
are |
|
Both |
Store the initial context in result types and stop waiting on context done with ErrRequestCanceled by giving all result types a context field and having receiveFuture stop waiting on context done with an ErrRequestCanceled. Result types have been changed from a channel of *response to a new cmdRes type that can hold the context. Remove duplicate function futureError and unused sendCmdAndWait.
There was a problem hiding this comment.
I've tested this pretty well and everything seems to be working as expected.
In addition to just general testing in the normal positive paths, I particularly focused testing on context cancellation at various points as follows:
- Before the call has been sent to the server
- After the call has been sent to the server, but before the response has been received
- After the response has been received from the server
One thing to keep in mind is that because the async receive machinery is selecting across the context and the response channel and Go has fair random channel selection, it is a coin flip as to whether or not the call to Receive will succeed in the case the context is cancelled when the response has already been received from the server.
I personally think this is both acceptable and appropriate behavior since the caller really shouldn't be trying to read a response after cancelling the context anyway, but I thought it was worth mentioning since any addition of tests around this behavior would need to account for that possibility.
rpcclient: Stop client on ctx done.
The passed context was ignored until a reconnect. This kills the client
when the passed context is done. This is much more useful for consumers
who need the clien to shut down when their context if finished.
rpcclient: Add a lifetime to requests.
Requests over websocket would block indefintely waiting for a response.
This adds a way for the caller to cancel requests, such as using a
context with deadline.