From 5ddb13bcef8fac9704b6e89cc3d6815871a379c0 Mon Sep 17 00:00:00 2001 From: Chris Pride Date: Sun, 17 Apr 2022 17:33:15 -0700 Subject: [PATCH] Fix websocket subscriptions to not double close. (#2095) We were closing at the end of the loop and also in the defer. Co-authored-by: Chris Pride --- graphql/handler/transport/websocket.go | 6 +----- graphql/handler/transport/websocket_test.go | 13 +++++++++++++ 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/graphql/handler/transport/websocket.go b/graphql/handler/transport/websocket.go index 28480a4092..a0429b7481 100644 --- a/graphql/handler/transport/websocket.go +++ b/graphql/handler/transport/websocket.go @@ -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 }() } diff --git a/graphql/handler/transport/websocket_test.go b/graphql/handler/transport/websocket_test.go index 02652b29b2..7c84f35262 100644 --- a/graphql/handler/transport/websocket_test.go +++ b/graphql/handler/transport/websocket_test.go @@ -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") + } + }) }