fix(coderd): Subscribe before sending initial metadata event

mafredri · mafredri · commit 2a53d6ac63a2 · 2023-06-13T12:14:12.000Z
diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go
@@ -1434,17 +1434,15 @@ func (api *API) watchWorkspaceAgentMetadata(rw http.ResponseWriter, r *http.Requ
 		})
 	}
 
-	// Send initial metadata.
-	sendMetadata(true)
-
 	// We debounce metadata updates to avoid overloading the frontend when
 	// an agent is sending a lot of updates.
 	pubsubDebounce := debounce.New(time.Second)
 	if flag.Lookup("test.v") != nil {
 		pubsubDebounce = debounce.New(time.Millisecond * 100)
 	}
 
-	// Send metadata on updates.
+	// Send metadata on updates, we must ensure subscription before sending
+	// initial metadata to guarantee that events in-between are not missed.
 	cancelSub, err := api.Pubsub.Subscribe(watchWorkspaceAgentMetadataChannel(workspaceAgent.ID), func(_ context.Context, _ []byte) {
 		pubsubDebounce(func() {
 			sendMetadata(true)
@@ -1456,6 +1454,9 @@ func (api *API) watchWorkspaceAgentMetadata(rw http.ResponseWriter, r *http.Requ
 	}
 	defer cancelSub()
 
+	// Send initial metadata.
+	sendMetadata(true)
+
 	for {
 		select {
 		case <-senderClosed:
diff --git a/coderd/workspaceagents_test.go b/coderd/workspaceagents_test.go
@@ -1290,10 +1290,10 @@ func TestWorkspaceAgent_Metadata(t *testing.T) {
 		return nil
 	}
 
-	check := func(want codersdk.WorkspaceAgentMetadataResult, got codersdk.WorkspaceAgentMetadata) {
+	check := func(want codersdk.WorkspaceAgentMetadataResult, got codersdk.WorkspaceAgentMetadata, retry bool) {
 		// We can't trust the order of the updates due to timers and debounces,
-		// so let's check a few times once more.
-		for i := 0; i < 2 && (want.Value != got.Result.Value || want.Error != got.Result.Error); i++ {
+		// so let's check a few times more.
+		for i := 0; retry && i < 2 && (want.Value != got.Result.Value || want.Error != got.Result.Error); i++ {
 			update = recvUpdate()
 			for _, m := range update {
 				if m.Description.Key == got.Description.Key {
@@ -1311,22 +1311,22 @@ func TestWorkspaceAgent_Metadata(t *testing.T) {
 
 	update = recvUpdate()
 	require.Len(t, update, 3)
-	check(wantMetadata1, update[0])
+	check(wantMetadata1, update[0], false)
 	// The second metadata result is not yet posted.
 	require.Zero(t, update[1].Result.CollectedAt)
 
 	wantMetadata2 := wantMetadata1
 	post("foo2", wantMetadata2)
 	update = recvUpdate()
 	require.Len(t, update, 3)
-	check(wantMetadata1, update[0])
-	check(wantMetadata2, update[1])
+	check(wantMetadata1, update[0], true)
+	check(wantMetadata2, update[1], true)
 
 	wantMetadata1.Error = "error"
 	post("foo1", wantMetadata1)
 	update = recvUpdate()
 	require.Len(t, update, 3)
-	check(wantMetadata1, update[0])
+	check(wantMetadata1, update[0], true)
 
 	const maxValueLen = 32 << 10
 	tooLongValueMetadata := wantMetadata1
@@ -1335,7 +1335,7 @@ func TestWorkspaceAgent_Metadata(t *testing.T) {
 	tooLongValueMetadata.CollectedAt = time.Now()
 	post("foo3", tooLongValueMetadata)
 	got := recvUpdate()[2]
-	for len(got.Result.Value) != maxValueLen {
+	for i := 0; i < 2 && len(got.Result.Value) != maxValueLen; i++ {
 		got = recvUpdate()[2]
 	}
 	require.Len(t, got.Result.Value, maxValueLen)
