Skip to content

chore: create interface for pkgs to return codersdk errors #18719

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
129 changes: 129 additions & 0 deletions coderd/dynamicparameters/error.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
package dynamicparameters

import (
"fmt"
"net/http"
"sort"

"github.com/hashicorp/hcl/v2"

"github.com/coder/coder/v2/codersdk"
)

func ParameterValidationError(diags hcl.Diagnostics) *DiagnosticError {
return &DiagnosticError{
Message: "Unable to validate parameters",
Diagnostics: diags,
KeyedDiagnostics: make(map[string]hcl.Diagnostics),
}
}

func TagValidationError(diags hcl.Diagnostics) *DiagnosticError {
return &DiagnosticError{
Message: "Failed to parse workspace tags",
Diagnostics: diags,
KeyedDiagnostics: make(map[string]hcl.Diagnostics),
}
}

type DiagnosticError struct {
// Message is the human-readable message that will be returned to the user.
Message string
// Diagnostics are top level diagnostics that will be returned as "Detail" in the response.
Diagnostics hcl.Diagnostics
// KeyedDiagnostics translate to Validation errors in the response. A key could
// be a parameter name, or a tag name. This allows diagnostics to be more closely
// associated with a specific index/parameter/tag.
KeyedDiagnostics map[string]hcl.Diagnostics
}

// Error is a pretty bad format for these errors. Try to avoid using this.
func (e *DiagnosticError) Error() string {
var diags hcl.Diagnostics
diags = diags.Extend(e.Diagnostics)
for _, d := range e.KeyedDiagnostics {
diags = diags.Extend(d)
}

return diags.Error()
}

func (e *DiagnosticError) HasError() bool {
if e.Diagnostics.HasErrors() {
return true
}

for _, diags := range e.KeyedDiagnostics {
if diags.HasErrors() {
return true
}
}
return false
}

func (e *DiagnosticError) Append(key string, diag *hcl.Diagnostic) {
e.Extend(key, hcl.Diagnostics{diag})
}

func (e *DiagnosticError) Extend(key string, diag hcl.Diagnostics) {
if e.KeyedDiagnostics == nil {
e.KeyedDiagnostics = make(map[string]hcl.Diagnostics)
}
if _, ok := e.KeyedDiagnostics[key]; !ok {
e.KeyedDiagnostics[key] = hcl.Diagnostics{}
}
e.KeyedDiagnostics[key] = e.KeyedDiagnostics[key].Extend(diag)
}

func (e *DiagnosticError) Response() (int, codersdk.Response) {
resp := codersdk.Response{
Message: e.Message,
Validations: nil,
}

// Sort the parameter names so that the order is consistent.
sortedNames := make([]string, 0, len(e.KeyedDiagnostics))
for name := range e.KeyedDiagnostics {
sortedNames = append(sortedNames, name)
}
sort.Strings(sortedNames)

for _, name := range sortedNames {
diag := e.KeyedDiagnostics[name]
resp.Validations = append(resp.Validations, codersdk.ValidationError{
Field: name,
Detail: DiagnosticsErrorString(diag),
})
}

if e.Diagnostics.HasErrors() {
resp.Detail = DiagnosticsErrorString(e.Diagnostics)
}

return http.StatusBadRequest, resp
}

func DiagnosticErrorString(d *hcl.Diagnostic) string {
return fmt.Sprintf("%s; %s", d.Summary, d.Detail)
}

func DiagnosticsErrorString(d hcl.Diagnostics) string {
count := len(d)
switch {
case count == 0:
return "no diagnostics"
case count == 1:
return DiagnosticErrorString(d[0])
default:
for _, d := range d {
// Render the first error diag.
// If there are warnings, do not priority them over errors.
if d.Severity == hcl.DiagError {
return fmt.Sprintf("%s, and %d other diagnostic(s)", DiagnosticErrorString(d), count-1)
}
}

// All warnings? ok...
return fmt.Sprintf("%s, and %d other diagnostic(s)", DiagnosticErrorString(d[0]), count-1)
}
}
51 changes: 3 additions & 48 deletions coderd/dynamicparameters/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,45 +26,6 @@ type parameterValue struct {
Source parameterValueSource
}

type ResolverError struct {
Diagnostics hcl.Diagnostics
Parameter map[string]hcl.Diagnostics
}

// Error is a pretty bad format for these errors. Try to avoid using this.
func (e *ResolverError) Error() string {
var diags hcl.Diagnostics
diags = diags.Extend(e.Diagnostics)
for _, d := range e.Parameter {
diags = diags.Extend(d)
}

return diags.Error()
}

func (e *ResolverError) HasError() bool {
if e.Diagnostics.HasErrors() {
return true
}

for _, diags := range e.Parameter {
if diags.HasErrors() {
return true
}
}
return false
}

func (e *ResolverError) Extend(parameterName string, diag hcl.Diagnostics) {
if e.Parameter == nil {
e.Parameter = make(map[string]hcl.Diagnostics)
}
if _, ok := e.Parameter[parameterName]; !ok {
e.Parameter[parameterName] = hcl.Diagnostics{}
}
e.Parameter[parameterName] = e.Parameter[parameterName].Extend(diag)
}

//nolint:revive // firstbuild is a control flag to turn on immutable validation
func ResolveParameters(
ctx context.Context,
Expand Down Expand Up @@ -112,10 +73,7 @@ func ResolveParameters(
// always be valid. If there is a case where this is not true, then this has to
// be changed to allow the build to continue with a different set of values.

return nil, &ResolverError{
Diagnostics: diags,
Parameter: nil,
}
return nil, ParameterValidationError(diags)
}

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

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

Expand Down
19 changes: 19 additions & 0 deletions coderd/httpapi/httperror/responserror.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package httperror

import (
"errors"

"github.com/coder/coder/v2/codersdk"
)

type Responder interface {
Response() (int, codersdk.Response)
}

func IsResponder(err error) (Responder, bool) {
var responseErr Responder
if errors.As(err, &responseErr) {
return responseErr, true
}
return nil, false
}
74 changes: 3 additions & 71 deletions coderd/httpapi/httperror/wsbuild.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,60 +2,17 @@ package httperror

import (
"context"
"errors"
"fmt"
"net/http"
"sort"

"github.com/hashicorp/hcl/v2"

"github.com/coder/coder/v2/coderd/dynamicparameters"
"github.com/coder/coder/v2/coderd/httpapi"
"github.com/coder/coder/v2/coderd/wsbuilder"
"github.com/coder/coder/v2/codersdk"
)

func WriteWorkspaceBuildError(ctx context.Context, rw http.ResponseWriter, err error) {
var buildErr wsbuilder.BuildError
if errors.As(err, &buildErr) {
if httpapi.IsUnauthorizedError(err) {
buildErr.Status = http.StatusForbidden
}

httpapi.Write(ctx, rw, buildErr.Status, codersdk.Response{
Message: buildErr.Message,
Detail: buildErr.Error(),
})
return
}

var parameterErr *dynamicparameters.ResolverError
if errors.As(err, &parameterErr) {
resp := codersdk.Response{
Message: "Unable to validate parameters",
Validations: nil,
}

// Sort the parameter names so that the order is consistent.
sortedNames := make([]string, 0, len(parameterErr.Parameter))
for name := range parameterErr.Parameter {
sortedNames = append(sortedNames, name)
}
sort.Strings(sortedNames)

for _, name := range sortedNames {
diag := parameterErr.Parameter[name]
resp.Validations = append(resp.Validations, codersdk.ValidationError{
Field: name,
Detail: DiagnosticsErrorString(diag),
})
}
if responseErr, ok := IsResponder(err); ok {
code, resp := responseErr.Response()

if parameterErr.Diagnostics.HasErrors() {
resp.Detail = DiagnosticsErrorString(parameterErr.Diagnostics)
}

httpapi.Write(ctx, rw, http.StatusBadRequest, resp)
httpapi.Write(ctx, rw, code, resp)
return
}

Expand All @@ -64,28 +21,3 @@ func WriteWorkspaceBuildError(ctx context.Context, rw http.ResponseWriter, err e
Detail: err.Error(),
})
}

func DiagnosticError(d *hcl.Diagnostic) string {
return fmt.Sprintf("%s; %s", d.Summary, d.Detail)
}

func DiagnosticsErrorString(d hcl.Diagnostics) string {
count := len(d)
switch {
case count == 0:
return "no diagnostics"
case count == 1:
return DiagnosticError(d[0])
default:
for _, d := range d {
// Render the first error diag.
// If there are warnings, do not priority them over errors.
if d.Severity == hcl.DiagError {
return fmt.Sprintf("%s, and %d other diagnostic(s)", DiagnosticError(d), count-1)
}
}

// All warnings? ok...
return fmt.Sprintf("%s, and %d other diagnostic(s)", DiagnosticError(d[0]), count-1)
}
}
10 changes: 10 additions & 0 deletions coderd/wsbuilder/wsbuilder.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,13 +240,23 @@ type BuildError struct {
}

func (e BuildError) Error() string {
if e.Wrapped == nil {
return e.Message
}
return e.Wrapped.Error()
}

func (e BuildError) Unwrap() error {
return e.Wrapped
}

func (e BuildError) Response() (int, codersdk.Response) {
return e.Status, codersdk.Response{
Message: e.Message,
Detail: e.Error(),
}
}

// Build computes and inserts a new workspace build into the database. If authFunc is provided, it also performs
// authorization preflight checks.
func (b *Builder) Build(
Expand Down
18 changes: 18 additions & 0 deletions coderd/wsbuilder/wsbuilder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

"github.com/coder/coder/v2/coderd/coderdtest"
"github.com/coder/coder/v2/coderd/files"
"github.com/coder/coder/v2/coderd/httpapi/httperror"
"github.com/coder/coder/v2/provisionersdk"

"github.com/google/uuid"
Expand Down Expand Up @@ -1000,6 +1001,23 @@ func TestWorkspaceBuildDeleteOrphan(t *testing.T) {
})
}

func TestWsbuildError(t *testing.T) {
t.Parallel()

const msg = "test error"
var buildErr error = wsbuilder.BuildError{
Status: http.StatusBadRequest,
Message: msg,
}

respErr, ok := httperror.IsResponder(buildErr)
require.True(t, ok, "should be a Coder SDK error")

code, resp := respErr.Response()
require.Equal(t, http.StatusBadRequest, code)
require.Equal(t, msg, resp.Message)
}

type txExpect func(mTx *dbmock.MockStore)

func expectDB(t *testing.T, opts ...txExpect) *dbmock.MockStore {
Expand Down
Loading