Skip to content

Commit

Permalink
Add case to redirect error responses early
Browse files Browse the repository at this point in the history
If we receive an error response with an id, but not sessionId, there's
a slim chance that this response should be redirected to a session
but will instead be redirected to the connection's event loop. This
means that there is a chance that we could end up with a handler that
is waiting for a response indefinitely (or until a timeout occurs).

Since we cannot know exactly which session the response should be
redirected to, we're sending it to all live sessions as well as the
connection's event loop.

Most handlers should ignore the extra error message, and the handler
waiting for the msg with the unique msgId will action on it. If no
handlers are alive or the session has already been closed that this
response msg should go to, then no handlers will action on it either.
  • Loading branch information
ankur22 committed Jul 27, 2023
1 parent 9e8919c commit 9cdfb46
Showing 1 changed file with 22 additions and 0 deletions.
22 changes: 22 additions & 0 deletions common/connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,28 @@ func (c *Connection) recvLoop() {
return
}

// In some cases the error response will only have the id and error,
// but no sessionId. In these cases we can't guarantee the origin of
// the request and so where the msg should be redirected to. To ensure
// the msg gets to the correct handler (which is potentially blocking
// a test iteration) we need to send it to all sessions and the
// connection's event loop.
case msg.ID != 0 && msg.Error != nil && msg.SessionID == "":
c.sessionsMu.RLock()
for _, s := range c.sessions {
select {
case s.readCh <- &msg:
case code := <-c.closeCh:
c.logger.Debugf("Connection:recvLoop:msg.ID:msg.Error:<-c.closeCh", "sid:%v tid:%v wsURL:%v crashed:%t", s.id, s.targetID, c.wsURL, s.crashed)

Check failure on line 383 in common/connection.go

View workflow job for this annotation

GitHub Actions / lint

line is 147 characters (lll)
_ = c.close(code)
case <-c.done:
c.logger.Debugf("Connection:recvLoop:msg.ID:msg.Error:<-c.done", "sid:%v tid:%v wsURL:%v crashed:%t", s.id, s.targetID, c.wsURL, s.crashed)

Check failure on line 386 in common/connection.go

View workflow job for this annotation

GitHub Actions / lint

line is 144 characters (lll)
return
}
}
c.sessionsMu.RUnlock()
c.emit("", &msg)

case msg.Method != "":
c.logger.Debugf("Connection:recvLoop:msg.Method:emit", "sid:%v method:%q", msg.SessionID, msg.Method)
ev, err := cdproto.UnmarshalMessage(&msg)
Expand Down

0 comments on commit 9cdfb46

Please sign in to comment.