Skip to content

Commit

Permalink
Fix websocket subscriptions to not double close. (#2095)
Browse files Browse the repository at this point in the history
We were closing at the end of the loop and also in the defer.

Co-authored-by: Chris Pride <cpride@observeinc.com>
  • Loading branch information
telemenar and Chris Pride authored Apr 18, 2022
1 parent a15a9bf commit 5a49764
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 5 deletions.
6 changes: 1 addition & 5 deletions graphql/handler/transport/websocket.go
Original file line number Diff line number Diff line change
Expand Up @@ -358,12 +358,8 @@ func (c *wsConnection) subscribe(start time.Time, msg *message) {

c.sendResponse(msg.id, response)
}
c.complete(msg.id)

c.mu.Lock()
delete(c.active, msg.id)
c.mu.Unlock()
cancel()
// complete and context cancel comes from the defer
}()
}

Expand Down
13 changes: 13 additions & 0 deletions graphql/handler/transport/websocket_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,19 @@ func TestWebsocket(t *testing.T) {
msg = readOp(c)
require.Equal(t, completeMsg, msg.Type)
require.Equal(t, "test_1", msg.ID)

// At this point we should be done and should not receive another message.
c.SetReadDeadline(time.Now().UTC().Add(1 * time.Millisecond))

err := c.ReadJSON(&msg)
if err == nil {
// This should not send a second close message for the same id.
require.NotEqual(t, completeMsg, msg.Type)
require.NotEqual(t, "test_1", msg.ID)
} else {
assert.Contains(t, err.Error(), "timeout")
}

})
}

Expand Down

0 comments on commit 5a49764

Please sign in to comment.