small bit of cleanup

coder · aslilac · Apr 10, 2025 · Apr 7, 2025 · Apr 8, 2025 · Apr 9, 2025
commit d28083bf155ef2f7a47d93e89ac8e68d908877cf
diff --git a/coderd/templateversions.go b/coderd/templateversions.go
@@ -283,6 +283,10 @@ func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http
 
 	// Check that the job has completed successfully
 	job, err := api.Database.GetProvisionerJobByID(ctx, templateVersion.JobID)
+	if httpapi.Is404Error(err) {
+		httpapi.ResourceNotFound(rw)
+		return
+	}
 	if err != nil {
 		httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
 			Message: "Internal error fetching provisioner job.",
@@ -292,7 +296,7 @@ func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http
 	}
 	if !job.CompletedAt.Valid {
 		httpapi.Write(ctx, rw, http.StatusTooEarly, codersdk.Response{
-			Message: "Job hasn't completed!",
+			Message: "Template version job has not finished",
 		})
 		return
 	}
@@ -309,7 +313,8 @@ func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http
 	input := preview.Input{
 		PlanJSON:        plan,
 		ParameterValues: map[string]string{},
-		// TODO: fill this out
+		// TODO: write a db query that fetches all of the data needed to fill out
+		// this owner value
 		Owner: previewtypes.WorkspaceOwner{
 			Groups: []string{"Everyone"},
 		},
@@ -357,7 +362,11 @@ func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http
 	if result != nil {
 		response.Parameters = result.Parameters
 	}
-	_ = stream.Send(response)
+	err = stream.Send(response)
+	if err != nil {
+		stream.Drop()
+		return
+	}
 
 	// As the user types into the form, reprocess the state using their input,
 	// and respond with updates.
@@ -367,7 +376,11 @@ func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http
 		case <-ctx.Done():
 			stream.Close(websocket.StatusGoingAway)
 			return
-		case update := <-updates:
+		case update, ok := <-updates:
+			if !ok {
+				// The connection has been closed, so there is no one to write to
+				return
+			}
 			input.ParameterValues = update.Inputs
 			result, diagnostics := preview.Preview(ctx, input, fs)
 			response := codersdk.DynamicParametersResponse{
@@ -377,7 +390,11 @@ func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http
 			if result != nil {
 				response.Parameters = result.Parameters
 			}
-			_ = stream.Send(response)
+			err = stream.Send(response)
+			if err != nil {
+				stream.Drop()
+				return
+			}
 		}
 	}
 }
@@ -404,7 +421,7 @@ func (api *API) templateVersionRichParameters(rw http.ResponseWriter, r *http.Re
 	}
 	if !job.CompletedAt.Valid {
 		httpapi.Write(ctx, rw, http.StatusTooEarly, codersdk.Response{
-			Message: "Job hasn't completed!",
+			Message: "Template version job has not finished",
 		})
 		return
 	}
@@ -544,7 +561,7 @@ func (api *API) templateVersionVariables(rw http.ResponseWriter, r *http.Request
 	}
 	if !job.CompletedAt.Valid {
 		httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{
-			Message: "Job hasn't completed!",
+			Message: "Template version job has not finished",
 		})
 		return
 	}

diff --git a/coderd/templateversions_test.go b/coderd/templateversions_test.go
@@ -2294,10 +2294,11 @@ func TestTemplateVersionDynamicParameters(t *testing.T) {
 	require.Equal(t, "Everyone", preview.Parameters[0].Value.Value.AsString())
 
 	// Send a new value, and see it reflected
-	stream.Send(codersdk.DynamicParametersRequest{
+	err = stream.Send(codersdk.DynamicParametersRequest{
 		ID:     1,
 		Inputs: map[string]string{"group": "Bloob"},
 	})
+	require.NoError(t, err)
 	preview = testutil.RequireRecvCtx(ctx, t, previews)
 	require.Equal(t, 1, preview.ID)
 	require.Empty(t, preview.Diagnostics)
@@ -2306,10 +2307,11 @@ func TestTemplateVersionDynamicParameters(t *testing.T) {
 	require.Equal(t, "Bloob", preview.Parameters[0].Value.Value.AsString())
 
 	// Back to default
-	stream.Send(codersdk.DynamicParametersRequest{
+	err = stream.Send(codersdk.DynamicParametersRequest{
 		ID:     3,
 		Inputs: map[string]string{},
 	})
+	require.NoError(t, err)
 	preview = testutil.RequireRecvCtx(ctx, t, previews)
 	require.Equal(t, 3, preview.ID)
 	require.Empty(t, preview.Diagnostics)

diff --git a/codersdk/wsjson/decoder.go b/codersdk/wsjson/decoder.go
@@ -18,9 +18,12 @@ type Decoder[T any] struct {
 	logger     slog.Logger
 }
 
-// Chan starts the decoder reading from the websocket and returns a channel for reading the
-// resulting values. The chan T is closed if the underlying websocket is closed, or we encounter an
-// error. We also close the underlying websocket if we encounter an error reading or decoding.
+// Chan returns a `chan` that you can read incoming messages from. The returned
+// `chan` will be closed when the WebSocket connection is closed. If there is an
+// error reading from the WebSocket or decoding a value the WebSocket will be
+// closed.
+//
+// Safety: Chan must only be called once. Successive calls will panic.
 func (d *Decoder[T]) Chan() <-chan T {
 	if !d.chanCalled.CompareAndSwap(false, true) {
 		panic("chan called more than once")

diff --git a/codersdk/wsjson/stream.go b/codersdk/wsjson/stream.go
@@ -6,9 +6,6 @@ import (
 )
 
 // Stream is a two-way messaging interface over a WebSocket connection.
-// As an implementation detail, we cannot currently use Encoder to implement
-// the writing side of things because it only supports sending one message, and
-// then immediately closing the WebSocket.
 type Stream[R any, W any] struct {
 	conn *websocket.Conn
 	r    *Decoder[R]
@@ -24,6 +21,12 @@ func NewStream[R any, W any](conn *websocket.Conn, readType, writeType websocket
 	}
 }
 
+// Chan returns a `chan` that you can read incoming messages from. The returned
+// `chan` will be closed when the WebSocket connection is closed. If there is an
+// error reading from the WebSocket or decoding a value the WebSocket will be
+// closed.
+//
+// Safety: Chan must only be called once. Successive calls will panic.
 func (s *Stream[R, W]) Chan() <-chan R {
 	return s.r.Chan()
 }
@@ -35,3 +38,7 @@ func (s *Stream[R, W]) Send(v W) error {
 func (s *Stream[R, W]) Close(c websocket.StatusCode) error {
 	return s.conn.Close(c, "")
 }
+
+func (s *Stream[R, W]) Drop() {
+	_ = s.conn.CloseNow()
+}