-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
fix issue #127 #1155
fix issue #127 #1155
Conversation
@@ -269,6 +281,8 @@ type rpcFunc struct { | |||
|
|||
hasCtx int | |||
retCh bool | |||
|
|||
retry bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We save here if was tagged with retry.
} | ||
|
||
var defaultConfig = Config{ | ||
ReconnectInterval: time.Second * 5, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Arbitrary...
|
||
type Option func(c *Config) | ||
|
||
func WithReconnectInterval(d time.Duration) func(c *Config) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If for some reason wants to change.
if req.req.ID != nil { | ||
if c.incomingErr != nil { // No conn?, immediate fail |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interestingly, whenever gorilla detects the connection is closed, it closes c.incoming
. Didn't expect that but found it while making some tests. So we never assume after reading <-c.requests
that we have a good conn. c.incomingErr
is set again to nil
when conn is reestablished (line 490)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks quite good, like it! Just 2 comments, then LGTM
Signed-off-by: jsign <jsign.uy@gmail.com>
Points addressed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a ton for this!
Signed-off-by: jsign <jsign.uy@gmail.com>
Closes #127
This is a continuation of #1146, since the main idea of the other PR changed.
There's a new tag available
retry:"true"
.e.g"
It works the same if the return value is a chan or not.
The meaning is: the function call will not return with failure on a closed websocket channel. It will keep retrying while the connection is trying to be reestablished.
If the connection is closed before
sendRequest
; saying it differently, the method is called with a closed connection:err
of closed connectionIf the connection is closed after the request was sent, but before received reply (in-flight):
err
When a connection is lost:
retry:"true"
cases, they will continue retryingTested with
ChainHead
andChainNotify
and seems to work correctly.