Skip to content

Commit d28083b

Browse files
aslilacaslilac
committed
small bit of cleanup
1 parent 15cd448 commit d28083b

File tree

4 files changed

+44
-15
lines changed

4 files changed

+44
-15
lines changed

‎coderd/templateversions.go

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,10 @@ func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http
283283

284284
// Check that the job has completed successfully
285285
job, err := api.Database.GetProvisionerJobByID(ctx, templateVersion.JobID)
286+
if httpapi.Is404Error(err) {
287+
httpapi.ResourceNotFound(rw)
288+
return
289+
}
286290
if err != nil {
287291
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
288292
Message: "Internal error fetching provisioner job.",
@@ -292,7 +296,7 @@ func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http
292296
}
293297
if !job.CompletedAt.Valid {
294298
httpapi.Write(ctx, rw, http.StatusTooEarly, codersdk.Response{
295-
Message: "Job hasn't completed!",
299+
Message: "Template version job has not finished",
296300
})
297301
return
298302
}
@@ -309,7 +313,8 @@ func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http
309313
input := preview.Input{
310314
PlanJSON: plan,
311315
ParameterValues: map[string]string{},
312-
// TODO: fill this out
316+
// TODO: write a db query that fetches all of the data needed to fill out
317+
// this owner value
313318
Owner: previewtypes.WorkspaceOwner{
314319
Groups: []string{"Everyone"},
315320
},
@@ -357,7 +362,11 @@ func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http
357362
if result != nil {
358363
response.Parameters = result.Parameters
359364
}
360-
_ = stream.Send(response)
365+
err = stream.Send(response)
366+
if err != nil {
367+
stream.Drop()
368+
return
369+
}
361370

362371
// As the user types into the form, reprocess the state using their input,
363372
// and respond with updates.
@@ -367,7 +376,11 @@ func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http
367376
case <-ctx.Done():
368377
stream.Close(websocket.StatusGoingAway)
369378
return
370-
case update := <-updates:
379+
case update, ok := <-updates:
380+
if !ok {
381+
// The connection has been closed, so there is no one to write to
382+
return
383+
}
371384
input.ParameterValues = update.Inputs
372385
result, diagnostics := preview.Preview(ctx, input, fs)
373386
response := codersdk.DynamicParametersResponse{
@@ -377,7 +390,11 @@ func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http
377390
if result != nil {
378391
response.Parameters = result.Parameters
379392
}
380-
_ = stream.Send(response)
393+
err = stream.Send(response)
394+
if err != nil {
395+
stream.Drop()
396+
return
397+
}
381398
}
382399
}
383400
}
@@ -404,7 +421,7 @@ func (api *API) templateVersionRichParameters(rw http.ResponseWriter, r *http.Re
404421
}
405422
if !job.CompletedAt.Valid {
406423
httpapi.Write(ctx, rw, http.StatusTooEarly, codersdk.Response{
407-
Message: "Job hasn't completed!",
424+
Message: "Template version job has not finished",
408425
})
409426
return
410427
}
@@ -544,7 +561,7 @@ func (api *API) templateVersionVariables(rw http.ResponseWriter, r *http.Request
544561
}
545562
if !job.CompletedAt.Valid {
546563
httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{
547-
Message: "Job hasn't completed!",
564+
Message: "Template version job has not finished",
548565
})
549566
return
550567
}

‎coderd/templateversions_test.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2294,10 +2294,11 @@ func TestTemplateVersionDynamicParameters(t *testing.T) {
22942294
require.Equal(t, "Everyone", preview.Parameters[0].Value.Value.AsString())
22952295

22962296
// Send a new value, and see it reflected
2297-
stream.Send(codersdk.DynamicParametersRequest{
2297+
err = stream.Send(codersdk.DynamicParametersRequest{
22982298
ID: 1,
22992299
Inputs: map[string]string{"group": "Bloob"},
23002300
})
2301+
require.NoError(t, err)
23012302
preview = testutil.RequireRecvCtx(ctx, t, previews)
23022303
require.Equal(t, 1, preview.ID)
23032304
require.Empty(t, preview.Diagnostics)
@@ -2306,10 +2307,11 @@ func TestTemplateVersionDynamicParameters(t *testing.T) {
23062307
require.Equal(t, "Bloob", preview.Parameters[0].Value.Value.AsString())
23072308

23082309
// Back to default
2309-
stream.Send(codersdk.DynamicParametersRequest{
2310+
err = stream.Send(codersdk.DynamicParametersRequest{
23102311
ID: 3,
23112312
Inputs: map[string]string{},
23122313
})
2314+
require.NoError(t, err)
23132315
preview = testutil.RequireRecvCtx(ctx, t, previews)
23142316
require.Equal(t, 3, preview.ID)
23152317
require.Empty(t, preview.Diagnostics)

‎codersdk/wsjson/decoder.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,12 @@ type Decoder[T any] struct {
1818
logger slog.Logger
1919
}
2020

21-
// Chan starts the decoder reading from the websocket and returns a channel for reading the
22-
// resulting values. The chan T is closed if the underlying websocket is closed, or we encounter an
23-
// error. We also close the underlying websocket if we encounter an error reading or decoding.
21+
// Chan returns a `chan` that you can read incoming messages from. The returned
22+
// `chan` will be closed when the WebSocket connection is closed. If there is an
23+
// error reading from the WebSocket or decoding a value the WebSocket will be
24+
// closed.
25+
//
26+
// Safety: Chan must only be called once. Successive calls will panic.
2427
func (d *Decoder[T]) Chan() <-chan T {
2528
if !d.chanCalled.CompareAndSwap(false, true) {
2629
panic("chan called more than once")

‎codersdk/wsjson/stream.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,6 @@ import (
66
)
77

88
// Stream is a two-way messaging interface over a WebSocket connection.
9-
// As an implementation detail, we cannot currently use Encoder to implement
10-
// the writing side of things because it only supports sending one message, and
11-
// then immediately closing the WebSocket.
129
type Stream[R any, W any] struct {
1310
conn *websocket.Conn
1411
r *Decoder[R]
@@ -24,6 +21,12 @@ func NewStream[R any, W any](conn *websocket.Conn, readType, writeType websocket
2421
}
2522
}
2623

24+
// Chan returns a `chan` that you can read incoming messages from. The returned
25+
// `chan` will be closed when the WebSocket connection is closed. If there is an
26+
// error reading from the WebSocket or decoding a value the WebSocket will be
27+
// closed.
28+
//
29+
// Safety: Chan must only be called once. Successive calls will panic.
2730
func (s *Stream[R, W]) Chan() <-chan R {
2831
return s.r.Chan()
2932
}
@@ -35,3 +38,7 @@ func (s *Stream[R, W]) Send(v W) error {
3538
func (s *Stream[R, W]) Close(c websocket.StatusCode) error {
3639
return s.conn.Close(c, "")
3740
}
41+
42+
func (s *Stream[R, W]) Drop() {
43+
_ = s.conn.CloseNow()
44+
}

0 commit comments

Comments
 (0)