Skip to content

Commit 15a6ca1

Browse files
committed
chore: interface for pkgs to return codersdk errors
1 parent 01163ea commit 15a6ca1

File tree

6 files changed

+175
-119
lines changed

6 files changed

+175
-119
lines changed

coderd/dynamicparameters/error.go

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
package dynamicparameters
2+
3+
import (
4+
"fmt"
5+
"net/http"
6+
"sort"
7+
8+
"github.com/hashicorp/hcl/v2"
9+
10+
"github.com/coder/coder/v2/codersdk"
11+
)
12+
13+
func ParameterValidationError(diags hcl.Diagnostics) *DiagnosticError {
14+
return &DiagnosticError{
15+
Message: "Unable to validate parameters",
16+
Diagnostics: diags,
17+
KeyedDiagnostics: make(map[string]hcl.Diagnostics),
18+
}
19+
}
20+
21+
func TagValidationError(diags hcl.Diagnostics) *DiagnosticError {
22+
return &DiagnosticError{
23+
Message: "Failed to parse workspace tags",
24+
Diagnostics: diags,
25+
KeyedDiagnostics: make(map[string]hcl.Diagnostics),
26+
}
27+
}
28+
29+
type DiagnosticError struct {
30+
Message string
31+
Diagnostics hcl.Diagnostics
32+
KeyedDiagnostics map[string]hcl.Diagnostics
33+
}
34+
35+
// Error is a pretty bad format for these errors. Try to avoid using this.
36+
func (e *DiagnosticError) Error() string {
37+
var diags hcl.Diagnostics
38+
diags = diags.Extend(e.Diagnostics)
39+
for _, d := range e.KeyedDiagnostics {
40+
diags = diags.Extend(d)
41+
}
42+
43+
return diags.Error()
44+
}
45+
46+
func (e *DiagnosticError) HasError() bool {
47+
if e.Diagnostics.HasErrors() {
48+
return true
49+
}
50+
51+
for _, diags := range e.KeyedDiagnostics {
52+
if diags.HasErrors() {
53+
return true
54+
}
55+
}
56+
return false
57+
}
58+
59+
func (e *DiagnosticError) Append(key string, diag *hcl.Diagnostic) {
60+
e.Extend(key, hcl.Diagnostics{diag})
61+
}
62+
63+
func (e *DiagnosticError) Extend(key string, diag hcl.Diagnostics) {
64+
if e.KeyedDiagnostics == nil {
65+
e.KeyedDiagnostics = make(map[string]hcl.Diagnostics)
66+
}
67+
if _, ok := e.KeyedDiagnostics[key]; !ok {
68+
e.KeyedDiagnostics[key] = hcl.Diagnostics{}
69+
}
70+
e.KeyedDiagnostics[key] = e.KeyedDiagnostics[key].Extend(diag)
71+
}
72+
73+
func (e *DiagnosticError) Response() (int, codersdk.Response) {
74+
resp := codersdk.Response{
75+
Message: e.Message,
76+
Validations: nil,
77+
}
78+
79+
// Sort the parameter names so that the order is consistent.
80+
sortedNames := make([]string, 0, len(e.KeyedDiagnostics))
81+
for name := range e.KeyedDiagnostics {
82+
sortedNames = append(sortedNames, name)
83+
}
84+
sort.Strings(sortedNames)
85+
86+
for _, name := range sortedNames {
87+
diag := e.KeyedDiagnostics[name]
88+
resp.Validations = append(resp.Validations, codersdk.ValidationError{
89+
Field: name,
90+
Detail: DiagnosticsErrorString(diag),
91+
})
92+
}
93+
94+
if e.Diagnostics.HasErrors() {
95+
resp.Detail = DiagnosticsErrorString(e.Diagnostics)
96+
}
97+
98+
return http.StatusBadRequest, resp
99+
}
100+
101+
func DiagnosticErrorString(d *hcl.Diagnostic) string {
102+
return fmt.Sprintf("%s; %s", d.Summary, d.Detail)
103+
}
104+
105+
func DiagnosticsErrorString(d hcl.Diagnostics) string {
106+
count := len(d)
107+
switch {
108+
case count == 0:
109+
return "no diagnostics"
110+
case count == 1:
111+
return DiagnosticErrorString(d[0])
112+
default:
113+
for _, d := range d {
114+
// Render the first error diag.
115+
// If there are warnings, do not priority them over errors.
116+
if d.Severity == hcl.DiagError {
117+
return fmt.Sprintf("%s, and %d other diagnostic(s)", DiagnosticErrorString(d), count-1)
118+
}
119+
}
120+
121+
// All warnings? ok...
122+
return fmt.Sprintf("%s, and %d other diagnostic(s)", DiagnosticErrorString(d[0]), count-1)
123+
}
124+
}

coderd/dynamicparameters/resolver.go

Lines changed: 3 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -26,45 +26,6 @@ type parameterValue struct {
2626
Source parameterValueSource
2727
}
2828

29-
type ResolverError struct {
30-
Diagnostics hcl.Diagnostics
31-
Parameter map[string]hcl.Diagnostics
32-
}
33-
34-
// Error is a pretty bad format for these errors. Try to avoid using this.
35-
func (e *ResolverError) Error() string {
36-
var diags hcl.Diagnostics
37-
diags = diags.Extend(e.Diagnostics)
38-
for _, d := range e.Parameter {
39-
diags = diags.Extend(d)
40-
}
41-
42-
return diags.Error()
43-
}
44-
45-
func (e *ResolverError) HasError() bool {
46-
if e.Diagnostics.HasErrors() {
47-
return true
48-
}
49-
50-
for _, diags := range e.Parameter {
51-
if diags.HasErrors() {
52-
return true
53-
}
54-
}
55-
return false
56-
}
57-
58-
func (e *ResolverError) Extend(parameterName string, diag hcl.Diagnostics) {
59-
if e.Parameter == nil {
60-
e.Parameter = make(map[string]hcl.Diagnostics)
61-
}
62-
if _, ok := e.Parameter[parameterName]; !ok {
63-
e.Parameter[parameterName] = hcl.Diagnostics{}
64-
}
65-
e.Parameter[parameterName] = e.Parameter[parameterName].Extend(diag)
66-
}
67-
6829
//nolint:revive // firstbuild is a control flag to turn on immutable validation
6930
func ResolveParameters(
7031
ctx context.Context,
@@ -112,10 +73,7 @@ func ResolveParameters(
11273
// always be valid. If there is a case where this is not true, then this has to
11374
// be changed to allow the build to continue with a different set of values.
11475

115-
return nil, &ResolverError{
116-
Diagnostics: diags,
117-
Parameter: nil,
118-
}
76+
return nil, ParameterValidationError(diags)
11977
}
12078

12179
// The user's input now needs to be validated against the parameters.
@@ -155,16 +113,13 @@ func ResolveParameters(
155113
// are fatal. Additional validation for immutability has to be done manually.
156114
output, diags = renderer.Render(ctx, ownerID, values.ValuesMap())
157115
if diags.HasErrors() {
158-
return nil, &ResolverError{
159-
Diagnostics: diags,
160-
Parameter: nil,
161-
}
116+
return nil, ParameterValidationError(diags)
162117
}
163118

164119
// parameterNames is going to be used to remove any excess values that were left
165120
// around without a parameter.
166121
parameterNames := make(map[string]struct{}, len(output.Parameters))
167-
parameterError := &ResolverError{}
122+
parameterError := ParameterValidationError(nil)
168123
for _, parameter := range output.Parameters {
169124
parameterNames[parameter.Name] = struct{}{}
170125

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
package httperror
2+
3+
import (
4+
"errors"
5+
6+
"github.com/coder/coder/v2/codersdk"
7+
)
8+
9+
type CoderSDKError interface {
10+
Response() (int, codersdk.Response)
11+
}
12+
13+
func IsCoderSDKError(err error) (CoderSDKError, bool) {
14+
var responseErr CoderSDKError
15+
if errors.As(err, &responseErr) {
16+
return responseErr, true
17+
}
18+
return nil, false
19+
}

coderd/httpapi/httperror/wsbuild.go

Lines changed: 3 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -2,60 +2,17 @@ package httperror
22

33
import (
44
"context"
5-
"errors"
6-
"fmt"
75
"net/http"
8-
"sort"
96

10-
"github.com/hashicorp/hcl/v2"
11-
12-
"github.com/coder/coder/v2/coderd/dynamicparameters"
137
"github.com/coder/coder/v2/coderd/httpapi"
14-
"github.com/coder/coder/v2/coderd/wsbuilder"
158
"github.com/coder/coder/v2/codersdk"
169
)
1710

1811
func WriteWorkspaceBuildError(ctx context.Context, rw http.ResponseWriter, err error) {
19-
var buildErr wsbuilder.BuildError
20-
if errors.As(err, &buildErr) {
21-
if httpapi.IsUnauthorizedError(err) {
22-
buildErr.Status = http.StatusForbidden
23-
}
24-
25-
httpapi.Write(ctx, rw, buildErr.Status, codersdk.Response{
26-
Message: buildErr.Message,
27-
Detail: buildErr.Error(),
28-
})
29-
return
30-
}
31-
32-
var parameterErr *dynamicparameters.ResolverError
33-
if errors.As(err, &parameterErr) {
34-
resp := codersdk.Response{
35-
Message: "Unable to validate parameters",
36-
Validations: nil,
37-
}
38-
39-
// Sort the parameter names so that the order is consistent.
40-
sortedNames := make([]string, 0, len(parameterErr.Parameter))
41-
for name := range parameterErr.Parameter {
42-
sortedNames = append(sortedNames, name)
43-
}
44-
sort.Strings(sortedNames)
45-
46-
for _, name := range sortedNames {
47-
diag := parameterErr.Parameter[name]
48-
resp.Validations = append(resp.Validations, codersdk.ValidationError{
49-
Field: name,
50-
Detail: DiagnosticsErrorString(diag),
51-
})
52-
}
12+
if responseErr, ok := IsCoderSDKError(err); ok {
13+
code, resp := responseErr.Response()
5314

54-
if parameterErr.Diagnostics.HasErrors() {
55-
resp.Detail = DiagnosticsErrorString(parameterErr.Diagnostics)
56-
}
57-
58-
httpapi.Write(ctx, rw, http.StatusBadRequest, resp)
15+
httpapi.Write(ctx, rw, code, resp)
5916
return
6017
}
6118

@@ -64,28 +21,3 @@ func WriteWorkspaceBuildError(ctx context.Context, rw http.ResponseWriter, err e
6421
Detail: err.Error(),
6522
})
6623
}
67-
68-
func DiagnosticError(d *hcl.Diagnostic) string {
69-
return fmt.Sprintf("%s; %s", d.Summary, d.Detail)
70-
}
71-
72-
func DiagnosticsErrorString(d hcl.Diagnostics) string {
73-
count := len(d)
74-
switch {
75-
case count == 0:
76-
return "no diagnostics"
77-
case count == 1:
78-
return DiagnosticError(d[0])
79-
default:
80-
for _, d := range d {
81-
// Render the first error diag.
82-
// If there are warnings, do not priority them over errors.
83-
if d.Severity == hcl.DiagError {
84-
return fmt.Sprintf("%s, and %d other diagnostic(s)", DiagnosticError(d), count-1)
85-
}
86-
}
87-
88-
// All warnings? ok...
89-
return fmt.Sprintf("%s, and %d other diagnostic(s)", DiagnosticError(d[0]), count-1)
90-
}
91-
}

coderd/wsbuilder/wsbuilder.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,13 +240,23 @@ type BuildError struct {
240240
}
241241

242242
func (e BuildError) Error() string {
243+
if e.Wrapped == nil {
244+
return e.Message
245+
}
243246
return e.Wrapped.Error()
244247
}
245248

246249
func (e BuildError) Unwrap() error {
247250
return e.Wrapped
248251
}
249252

253+
func (e BuildError) Response() (int, codersdk.Response) {
254+
return e.Status, codersdk.Response{
255+
Message: e.Message,
256+
Detail: e.Error(),
257+
}
258+
}
259+
250260
// Build computes and inserts a new workspace build into the database. If authFunc is provided, it also performs
251261
// authorization preflight checks.
252262
func (b *Builder) Build(

coderd/wsbuilder/wsbuilder_test.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212

1313
"github.com/coder/coder/v2/coderd/coderdtest"
1414
"github.com/coder/coder/v2/coderd/files"
15+
"github.com/coder/coder/v2/coderd/httpapi/httperror"
1516
"github.com/coder/coder/v2/provisionersdk"
1617

1718
"github.com/google/uuid"
@@ -1000,6 +1001,21 @@ func TestWorkspaceBuildDeleteOrphan(t *testing.T) {
10001001
})
10011002
}
10021003

1004+
func TestWsbuildError(t *testing.T) {
1005+
const msg = "test error"
1006+
var buildErr error = wsbuilder.BuildError{
1007+
Status: http.StatusBadRequest,
1008+
Message: msg,
1009+
}
1010+
1011+
respErr, ok := httperror.IsCoderSDKError(buildErr)
1012+
require.True(t, ok, "should be a Coder SDK error")
1013+
1014+
code, resp := respErr.Response()
1015+
require.Equal(t, http.StatusBadRequest, code)
1016+
require.Equal(t, resp.Message, msg)
1017+
}
1018+
10031019
type txExpect func(mTx *dbmock.MockStore)
10041020

10051021
func expectDB(t *testing.T, opts ...txExpect) *dbmock.MockStore {

0 commit comments

Comments
 (0)