don't close the connection when opening the identify stream fails #1293
Conversation
No functional change expected.
There was a problem hiding this comment.
I would also ask for @Stebalien 's opinion here as well, as this might have interesting effects up the stack.
|
|
||
| s, err = c.NewStream(network.WithUseTransient(context.TODO(), "identify")) | ||
| func (ids *idService) identifyConn(c network.Conn) error { | ||
| s, err := c.NewStream(network.WithUseTransient(context.TODO(), "identify")) |
There was a problem hiding this comment.
shouldnt we have a timeout here? Or do we set the deadline?
There was a problem hiding this comment.
Let's do that in a separate PR. We do set a bunch of deadlines, but only read deadlines, which can lead to funny edge cases if the peer withholds flow control credit.
|
Oh damn, I merged this PR accidentally. Sorry for that. I'm happy to revert / fix in a follow-up PR if necessary. |
|
thats fine, i was going to approve anyway. I still want @Stebalien 's opinion on this however. |
|
Currently, this will mark the function as "identified" instead of trying again in the future.
|
This is obviously very important once the rcmgr is used. This PR also cleans up some rather messy
deferlogic that was used for error handling.