feat: avoid error when subscription ID is not present (#387)

This PR removes the error triggered when a websocket message does not
contain a subscription ID.

Check this issue
https://github.com/issues?q=sort%3Aupdated-desc+is%3Aissue+is%3Aopen+author%3A%40me+archived%3Afalse&issue=Khan%7Cgenqlient%7C383

I have:
- [x] Written a clear PR title and description (above)
- [x] Signed the [Khan Academy CLA](https://www.khanacademy.org/r/cla)
- [x] Added tests covering my changes, if applicable
- [x] Included a link to the issue fixed, if applicable
- [ ] Included documentation, for new features
- [x] Added an entry to the changelog

---------

Co-authored-by: Ben Kraft <[email protected]>

Author: cheina97

docs/CHANGELOG.md

@@ -41,6 +41,7 @@ This release fixes a bug introduced in v0.8.0 breaking path resolution on Window
 - fixed documentation link in `introduction.md`
 - upgraded version of alexflint/go-arg from 1.4.2 to 1.5.1
 - fixed a typo in the struct + fragment error message
+- avoid error when a subscription message is received without a subscription ID
 - avoid closing subscription channels more than once, which could cause a panic in some cases
 
 ## v0.8.0

graphql/websocket.go

@@ -136,6 +136,9 @@ func (w *webSocketClient) forwardWebSocketData(message []byte) error {
 	if err != nil {
 		return err
 	}
+	if wsMsg.ID == "" { // e.g. keep-alive messages
+		return nil
+	}
 	sub, ok := w.subscriptions.Read(wsMsg.ID)
 	if !ok {
 		return fmt.Errorf("received message for unknown subscription ID '%s'", wsMsg.ID)

graphql/websocket_test.go

@@ -0,0 +1,92 @@
+package graphql
+
+import (
+	"encoding/json"
+	"sync"
+	"testing"
+)
+
+const testSubscriptionID = "test-subscription-id"
+
+func forgeTestWebSocketClient(hasBeenUnsubscribed bool) *webSocketClient {
+	return &webSocketClient{
+		subscriptions: subscriptionMap{
+			RWMutex: sync.RWMutex{},
+			map_: map[string]subscription{
+				testSubscriptionID: {
+					hasBeenUnsubscribed: hasBeenUnsubscribed,
+					interfaceChan:       make(chan any),
+					forwardDataFunc: func(interfaceChan any, jsonRawMsg json.RawMessage) error {
+						return nil
+					},
+				},
+			},
+		},
+	}
+}
+
+func Test_webSocketClient_forwardWebSocketData(t *testing.T) {
+	type args struct {
+		message []byte
+	}
+	tests := []struct {
+		wc      *webSocketClient
+		name    string
+		args    args
+		wantErr bool
+	}{
+		{
+			name:    "empty message",
+			args:    args{message: []byte{}},
+			wc:      forgeTestWebSocketClient(false),
+			wantErr: true,
+		},
+		{
+			name:    "nil message",
+			args:    args{message: nil},
+			wc:      forgeTestWebSocketClient(false),
+			wantErr: true,
+		},
+		{
+			name:    "unknown subscription id",
+			args:    args{message: []byte(`{"type":"next","id":"unknown-id","payload":{}}`)},
+			wc:      forgeTestWebSocketClient(false),
+			wantErr: true,
+		},
+		{
+			name:    "void subscription ID",
+			args:    args{message: []byte(`{"type":"next","id":"","payload":{}}`)},
+			wc:      forgeTestWebSocketClient(false),
+			wantErr: false,
+		},
+		{
+			name:    "unsubscribed subscription",
+			args:    args{message: []byte(`{"type":"next","id":"test-subscription-id","payload":{}}`)},
+			wc:      forgeTestWebSocketClient(true),
+			wantErr: false,
+		},
+		{
+			name:    "complete message closes channel",
+			args:    args{message: []byte(`{"type":"complete","id":"test-subscription-id","payload":{}}`)},
+			wc:      forgeTestWebSocketClient(false),
+			wantErr: false,
+		},
+		{
+			name:    "valid next message",
+			args:    args{message: []byte(`{"type":"next","id":"test-subscription-id","payload":{"foo":"bar"}}`)},
+			wc:      forgeTestWebSocketClient(false),
+			wantErr: false,
+		},
+	}
+	for i := range tests {
+		tt := &tests[i]
+		t.Run(tt.name, func(t *testing.T) {
+			t.Logf("Running test: %s", tt.name)
+
+			if err := tt.wc.forwardWebSocketData(tt.args.message); (err != nil) != tt.wantErr {
+				t.Errorf("%s: webSocketClient.forwardWebSocketData() error = %v, wantErr %v", tt.name, err, tt.wantErr)
+				return
+			}
+		})
+	}
+}