Skip to content

Commit 27ea415

Browse files
johnstcnjohnstcn
authored
fix: remove string TTL from workspace error responses (coder#3257)
- Rewrites some error messages to better integrate with the frontend (ttl_ms -> time until shutdown) - Makes codersdk.ValidationError implement the error interface - Only return validations if the error was a validation error, return detail otherwise (e.g. database error)
1 parent 36ffdce commit 27ea415

5 files changed

Lines changed: 34 additions & 27 deletions

File tree

‎coderd/workspaces.go‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req
288288
dbTTL, err := validWorkspaceTTLMillis(createWorkspace.TTLMillis, time.Duration(template.MaxTtl))
289289
if err != nil {
290290
httpapi.Write(rw, http.StatusBadRequest, codersdk.Response{
291-
Message: "Invalid Workspace TTL.",
291+
Message: "Invalid Workspace Time to Shutdown.",
292292
Validations: []codersdk.ValidationError{{Field: "ttl_ms", Detail: err.Error()}},
293293
})
294294
return
@@ -523,8 +523,6 @@ func (api *API) putWorkspaceTTL(rw http.ResponseWriter, r *http.Request) {
523523
return
524524
}
525525

526-
var validErrs []codersdk.ValidationError
527-
528526
err := api.Database.InTx(func(s database.Store) error {
529527
template, err := s.GetTemplateByID(r.Context(), workspace.TemplateID)
530528
if err != nil {
@@ -536,29 +534,31 @@ func (api *API) putWorkspaceTTL(rw http.ResponseWriter, r *http.Request) {
536534

537535
dbTTL, err := validWorkspaceTTLMillis(req.TTLMillis, time.Duration(template.MaxTtl))
538536
if err != nil {
539-
validErrs = append(validErrs, codersdk.ValidationError{Field: "ttl_ms", Detail: err.Error()})
540-
return err
537+
return codersdk.ValidationError{Field: "ttl_ms", Detail: err.Error()}
541538
}
542539
if err := s.UpdateWorkspaceTTL(r.Context(), database.UpdateWorkspaceTTLParams{
543540
ID: workspace.ID,
544541
Ttl: dbTTL,
545542
}); err != nil {
546-
return xerrors.Errorf("update workspace TTL: %w", err)
543+
return xerrors.Errorf("update workspace time until shutdown: %w", err)
547544
}
548545

549546
return nil
550547
})
551548

552549
if err != nil {
553-
code := http.StatusInternalServerError
554-
if len(validErrs) > 0 {
555-
code = http.StatusBadRequest
550+
resp := codersdk.Response{
551+
Message: "Error updating workspace time until shutdown.",
556552
}
557-
httpapi.Write(rw, code, codersdk.Response{
558-
Message: "Error updating workspace time until shutdown!",
559-
Validations: validErrs,
560-
Detail: err.Error(),
561-
})
553+
var validErr codersdk.ValidationError
554+
if errors.As(err, &validErr) {
555+
resp.Validations = []codersdk.ValidationError{validErr}
556+
httpapi.Write(rw, http.StatusBadRequest, resp)
557+
return
558+
}
559+
560+
resp.Detail = err.Error()
561+
httpapi.Write(rw, http.StatusInternalServerError, resp)
562562
return
563563
}
564564

@@ -895,15 +895,15 @@ func validWorkspaceTTLMillis(millis *int64, max time.Duration) (sql.NullInt64, e
895895
dur := time.Duration(*millis) * time.Millisecond
896896
truncated := dur.Truncate(time.Minute)
897897
if truncated < time.Minute {
898-
return sql.NullInt64{}, xerrors.New("ttl must be at least one minute")
898+
return sql.NullInt64{}, xerrors.New("time until shutdown must be at least one minute")
899899
}
900900

901901
if truncated > 24*7*time.Hour {
902-
return sql.NullInt64{}, xerrors.New("ttl must be less than 7 days")
902+
return sql.NullInt64{}, xerrors.New("time until shutdown must be less than 7 days")
903903
}
904904

905905
if truncated > max {
906-
return sql.NullInt64{}, xerrors.Errorf("ttl must be below template maximum %s", max.String())
906+
return sql.NullInt64{}, xerrors.Errorf("time until shutdown must be below template maximum %s", max.String())
907907
}
908908

909909
return sql.NullInt64{

‎coderd/workspaces_test.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ func TestPostWorkspacesByOrganization(t *testing.T) {
207207
require.Equal(t, http.StatusBadRequest, apiErr.StatusCode())
208208
require.Len(t, apiErr.Validations, 1)
209209
require.Equal(t, apiErr.Validations[0].Field, "ttl_ms")
210-
require.Equal(t, apiErr.Validations[0].Detail, "ttl must be at least one minute")
210+
require.Equal(t, "time until shutdown must be at least one minute", apiErr.Validations[0].Detail)
211211
})
212212

213213
t.Run("AboveMax", func(t *testing.T) {
@@ -220,7 +220,7 @@ func TestPostWorkspacesByOrganization(t *testing.T) {
220220
req := codersdk.CreateWorkspaceRequest{
221221
TemplateID: template.ID,
222222
Name: "testing",
223-
TTLMillis: ptr.Ref((24*7*time.Hour + time.Minute).Milliseconds()),
223+
TTLMillis: ptr.Ref(template.MaxTTLMillis + time.Minute.Milliseconds()),
224224
}
225225
_, err := client.CreateWorkspace(context.Background(), template.OrganizationID, req)
226226
require.Error(t, err)
@@ -229,7 +229,7 @@ func TestPostWorkspacesByOrganization(t *testing.T) {
229229
require.Equal(t, http.StatusBadRequest, apiErr.StatusCode())
230230
require.Len(t, apiErr.Validations, 1)
231231
require.Equal(t, apiErr.Validations[0].Field, "ttl_ms")
232-
require.Equal(t, apiErr.Validations[0].Detail, "ttl must be less than 7 days")
232+
require.Equal(t, "time until shutdown must be less than 7 days", apiErr.Validations[0].Detail)
233233
})
234234
})
235235

@@ -934,7 +934,7 @@ func TestWorkspaceUpdateTTL(t *testing.T) {
934934
{
935935
name: "below minimum ttl",
936936
ttlMillis: ptr.Ref((30 * time.Second).Milliseconds()),
937-
expectedError: "ttl must be at least one minute",
937+
expectedError: "time until shutdown must be at least one minute",
938938
},
939939
{
940940
name: "minimum ttl",
@@ -949,12 +949,12 @@ func TestWorkspaceUpdateTTL(t *testing.T) {
949949
{
950950
name: "above maximum ttl",
951951
ttlMillis: ptr.Ref((24*7*time.Hour + time.Minute).Milliseconds()),
952-
expectedError: "ttl must be less than 7 days",
952+
expectedError: "time until shutdown must be less than 7 days",
953953
},
954954
{
955955
name: "above template maximum ttl",
956956
ttlMillis: ptr.Ref((12 * time.Hour).Milliseconds()),
957-
expectedError: "ttl_ms: ttl must be below template maximum 8h0m0s",
957+
expectedError: "ttl_ms: time until shutdown must be below template maximum 8h0m0s",
958958
modifyTemplate: func(ctr *codersdk.CreateTemplateRequest) { ctr.MaxTTLMillis = ptr.Ref((8 * time.Hour).Milliseconds()) },
959959
},
960960
}

‎codersdk/error.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package codersdk
22

33
import (
4+
"fmt"
45
"net"
56

67
"golang.org/x/xerrors"
@@ -32,6 +33,12 @@ type ValidationError struct {
3233
Detail string `json:"detail" validate:"required"`
3334
}
3435

36+
func (e ValidationError) Error() string {
37+
return fmt.Sprintf("field: %s detail: %s", e.Field, e.Detail)
38+
}
39+
40+
var _ error = (*ValidationError)(nil)
41+
3542
// IsConnectionErr is a convenience function for checking if the source of an
3643
// error is due to a 'connection refused', 'no such host', etc.
3744
func IsConnectionErr(err error) bool {

‎codersdk/workspaces.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ func (c *Client) UpdateWorkspaceTTL(ctx context.Context, id uuid.UUID, req Updat
192192
path := fmt.Sprintf("/api/v2/workspaces/%s/ttl", id.String())
193193
res, err := c.Request(ctx, http.MethodPut, path, req)
194194
if err != nil {
195-
return xerrors.Errorf("update workspace ttl: %w", err)
195+
return xerrors.Errorf("update workspace time until shutdown: %w", err)
196196
}
197197
defer res.Body.Close()
198198
if res.StatusCode != http.StatusOK {
@@ -212,7 +212,7 @@ func (c *Client) PutExtendWorkspace(ctx context.Context, id uuid.UUID, req PutEx
212212
path := fmt.Sprintf("/api/v2/workspaces/%s/extend", id.String())
213213
res, err := c.Request(ctx, http.MethodPut, path, req)
214214
if err != nil {
215-
return xerrors.Errorf("extend workspace ttl: %w", err)
215+
return xerrors.Errorf("extend workspace time until shutdown: %w", err)
216216
}
217217
defer res.Body.Close()
218218
if res.StatusCode != http.StatusOK && res.StatusCode != http.StatusNotModified {

‎site/src/api/typesGenerated.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,7 @@ export interface PutExtendWorkspaceRequest {
250250
readonly deadline: string
251251
}
252252

253-
// From codersdk/error.go:10:6
253+
// From codersdk/error.go:11:6
254254
export interface Response {
255255
readonly message: string
256256
readonly detail?: string
@@ -386,7 +386,7 @@ export interface UsersRequest extends Pagination {
386386
readonly q?: string
387387
}
388388

389-
// From codersdk/error.go:30:6
389+
// From codersdk/error.go:31:6
390390
export interface ValidationError {
391391
readonly field: string
392392
readonly detail: string

0 commit comments

Comments
 (0)