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

This interface allows it to create rich codersdk errors and pass them up to the `wsbuilder` error handling.

coderd/dynamicparameters/error.go

@@ -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)
+	}
+}

coderd/dynamicparameters/resolver.go

@@ -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,
@@ -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.
@@ -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{}{}
 

coderd/httpapi/httperror/responserror.go

@@ -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
+}

coderd/httpapi/httperror/wsbuild.go

@@ -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
-		}
+	if responseErr, ok := IsResponder(err); ok {
+		code, resp := responseErr.Response()
 
-		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 parameterErr.Diagnostics.HasErrors() {
-			resp.Detail = DiagnosticsErrorString(parameterErr.Diagnostics)
-		}
-
-		httpapi.Write(ctx, rw, http.StatusBadRequest, resp)
+		httpapi.Write(ctx, rw, code, resp)
 		return
 	}
 
@@ -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)
-	}
-}

coderd/wsbuilder/wsbuilder.go

@@ -240,6 +240,9 @@ type BuildError struct {
 }
 
 func (e BuildError) Error() string {
+	if e.Wrapped == nil {
+		return e.Message
+	}
 	return e.Wrapped.Error()
 }
 
@@ -247,6 +250,13 @@ 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(

coderd/wsbuilder/wsbuilder_test.go

@@ -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"
@@ -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 {