fix: validation step errors return 4xx instead of 500 (closes #359)

Adds a ValidationError type in interfaces/ that HTTP handlers detect and
map to the appropriate 4xx status code instead of always returning 500.

- interfaces: add ValidationError, NewValidationError, IsValidationError,
  ValidationErrorStatus helpers
- config: add error_status field to PipelineStepConfig
- module: add ErrorStatusStep wrapper that converts step errors into
  ValidationErrors with the configured HTTP status
- engine: wire error_status wrapping in buildPipelineSteps alongside the
  existing skip_if / if guard wrappers
- module/http_trigger: detect ValidationError and respond with its status
  code (400, 422, …) and a JSON error body instead of 500

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: intel352

config/pipeline.go

@@ -30,4 +30,9 @@ type PipelineStepConfig struct {
 	// template evaluates to truthy. Falsy or absent with no SkipIf → execute.
 	// When both SkipIf and If are set, SkipIf takes precedence.
 	If string `json:"if,omitempty" yaml:"if,omitempty"`
+	// ErrorStatus overrides the HTTP response status code when this step fails.
+	// Use 400 for bad requests, 422 for unprocessable entity, etc.
+	// When set, the step error is wrapped in a ValidationError so the HTTP
+	// handler returns the specified status code instead of 500.
+	ErrorStatus int `json:"error_status,omitempty" yaml:"error_status,omitempty"`
 }

engine.go

@@ -1073,6 +1073,13 @@ func (e *StdEngine) buildPipelineSteps(pipelineName string, stepCfgs []config.Pi
 			step = module.NewSkippableStep(step, sc.SkipIf, sc.If)
 		}
 
+		// Wrap the step with an error_status mapper when set so that step
+		// failures are surfaced as ValidationErrors and the HTTP handler
+		// returns the configured 4xx status instead of 500.
+		if sc.ErrorStatus != 0 {
+			step = module.NewErrorStatusStep(step, sc.ErrorStatus)
+		}
+
 		steps = append(steps, step)
 	}
 

interfaces/pipeline.go

@@ -6,6 +6,7 @@ package interfaces
 
 import (
 	"context"
+	"errors"
 	"log/slog"
 	"maps"
 )
@@ -118,6 +119,42 @@ type PipelineStep interface {
 	Execute(ctx context.Context, pc *PipelineContext) (*StepResult, error)
 }
 
+// ValidationError indicates a step failed due to invalid user input, not an
+// infrastructure error. HTTP handlers map this to a 4xx status code instead
+// of the default 500 Internal Server Error.
+type ValidationError struct {
+	Message string
+	Status  int    // HTTP status code to use (e.g., 400, 422)
+	Field   string // optional: which field was invalid
+	Code    string // optional: machine-readable error code for clients
+}
+
+func (e *ValidationError) Error() string { return e.Message }
+
+// NewValidationError creates a ValidationError with the given message and HTTP
+// status code. Use status 400 for bad input, 422 for unprocessable entity, etc.
+func NewValidationError(msg string, status int) *ValidationError {
+	return &ValidationError{Message: msg, Status: status}
+}
+
+// IsValidationError reports whether err (or any error in its chain) is a
+// *ValidationError.
+func IsValidationError(err error) bool {
+	var ve *ValidationError
+	return errors.As(err, &ve)
+}
+
+// ValidationErrorStatus returns the HTTP status code from a ValidationError in
+// the error chain. Returns 400 if the error has no status or is not a
+// ValidationError.
+func ValidationErrorStatus(err error) int {
+	var ve *ValidationError
+	if errors.As(err, &ve) && ve.Status != 0 {
+		return ve.Status
+	}
+	return 400
+}
+
 // StepRegistrar manages step type registration and creation.
 // It embeds StepRegistryProvider for type enumeration and adds
 // a Create method for instantiating steps. Register is intentionally

module/http_trigger.go

@@ -13,6 +13,7 @@ import (
 	"strings"
 
 	"github.com/GoCodeAlone/modular"
+	"github.com/GoCodeAlone/workflow/interfaces"
 )
 
 // httpRWContextKey is the unexported type for the HTTP response writer context key.
@@ -436,6 +437,15 @@ func (t *HTTPTrigger) createHandler(route HTTPTriggerRoute) HTTPHandler {
 		// Call the workflow engine to trigger the workflow
 		err := t.engine.TriggerWorkflow(ctx, route.Workflow, route.Action, data)
 		if err != nil {
+			if interfaces.IsValidationError(err) {
+				status := interfaces.ValidationErrorStatus(err)
+				w.Header().Set("Content-Type", "application/json")
+				w.WriteHeader(status)
+				if encErr := json.NewEncoder(w).Encode(map[string]any{"error": err.Error()}); encErr != nil {
+					log.Printf("http trigger: failed to write validation error response: %v", encErr)
+				}
+				return
+			}
 			http.Error(w, fmt.Sprintf("Error triggering workflow: %v", err), http.StatusInternalServerError)
 			return
 		}

module/http_trigger_test.go

@@ -3,12 +3,14 @@ package module
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"strings"
 	"testing"
 
 	"github.com/GoCodeAlone/modular"
+	"github.com/GoCodeAlone/workflow/interfaces"
 )
 
 // TestHTTPTrigger tests the HTTP trigger functionality
@@ -625,3 +627,105 @@ func TestHTTPTrigger_PipelineOutput(t *testing.T) {
 		t.Errorf("expected status=active, got %v", body["status"])
 	}
 }
+
+// errorEngine is a test engine that always returns a configured error.
+type errorEngine struct {
+	err error
+}
+
+func (e *errorEngine) TriggerWorkflow(_ context.Context, _, _ string, _ map[string]any) error {
+	return e.err
+}
+
+// setupErrorEngineRoute is a helper that registers an errorEngine with the
+// HTTP trigger and returns the handler for the registered route.
+func setupErrorEngineRoute(t *testing.T, eng WorkflowEngine) HTTPHandler {
+	t.Helper()
+	app := NewMockApplication()
+	router := NewMockHTTPRouter("test-router")
+	_ = app.RegisterService("httpRouter", router)
+	_ = app.RegisterService("workflowEngine", eng)
+
+	trigger := NewHTTPTrigger()
+	app.RegisterModule(trigger)
+	cfg := map[string]any{
+		"routes": []any{
+			map[string]any{
+				"path":     "/api/validate",
+				"method":   "POST",
+				"workflow": "test-wf",
+				"action":   "execute",
+			},
+		},
+	}
+	if err := trigger.Configure(app, cfg); err != nil {
+		t.Fatalf("Configure: %v", err)
+	}
+	if err := trigger.Start(context.Background()); err != nil {
+		t.Fatalf("Start: %v", err)
+	}
+	h, ok := router.routes["POST /api/validate"]
+	if !ok {
+		t.Fatal("handler not registered")
+	}
+	return h
+}
+
+// TestHTTPHandler_ValidationError_Returns400 verifies that when a workflow step
+// returns a ValidationError with status 400, the HTTP trigger responds with 400
+// and a JSON body containing the error message, not 500.
+func TestHTTPHandler_ValidationError_Returns400(t *testing.T) {
+	valErr := interfaces.NewValidationError("invalid move: card doesn't match", 400)
+	eng := &errorEngine{err: valErr}
+	handler := setupErrorEngineRoute(t, eng)
+
+	req := httptest.NewRequest("POST", "/api/validate", nil)
+	w := httptest.NewRecorder()
+	handler.Handle(w, req)
+
+	resp := w.Result()
+	if resp.StatusCode != 400 {
+		t.Errorf("expected 400, got %d", resp.StatusCode)
+	}
+	if ct := w.Header().Get("Content-Type"); ct != "application/json" {
+		t.Errorf("expected Content-Type application/json, got %q", ct)
+	}
+	var body map[string]any
+	if err := json.NewDecoder(w.Body).Decode(&body); err != nil {
+		t.Fatalf("failed to decode response body: %v", err)
+	}
+	if body["error"] != "invalid move: card doesn't match" {
+		t.Errorf("unexpected error message: %v", body["error"])
+	}
+}
+
+// TestHTTPHandler_ValidationError_Returns422 verifies that a ValidationError
+// with status 422 causes the HTTP trigger to respond with 422.
+func TestHTTPHandler_ValidationError_Returns422(t *testing.T) {
+	valErr := interfaces.NewValidationError("unprocessable input", 422)
+	eng := &errorEngine{err: valErr}
+	handler := setupErrorEngineRoute(t, eng)
+
+	req := httptest.NewRequest("POST", "/api/validate", nil)
+	w := httptest.NewRecorder()
+	handler.Handle(w, req)
+
+	if w.Result().StatusCode != 422 {
+		t.Errorf("expected 422, got %d", w.Result().StatusCode)
+	}
+}
+
+// TestHTTPHandler_InfraError_Returns500 verifies that a non-ValidationError
+// still results in a 500 Internal Server Error response.
+func TestHTTPHandler_InfraError_Returns500(t *testing.T) {
+	eng := &errorEngine{err: fmt.Errorf("database connection failed")}
+	handler := setupErrorEngineRoute(t, eng)
+
+	req := httptest.NewRequest("POST", "/api/validate", nil)
+	w := httptest.NewRecorder()
+	handler.Handle(w, req)
+
+	if w.Result().StatusCode != 500 {
+		t.Errorf("expected 500, got %d", w.Result().StatusCode)
+	}
+}

module/pipeline_step_error_status.go

@@ -0,0 +1,40 @@
+package module
+
+import (
+	"context"
+
+	"github.com/GoCodeAlone/workflow/interfaces"
+)
+
+// ErrorStatusStep wraps a PipelineStep and converts any step error into a
+// ValidationError with a configured HTTP status code. This allows YAML pipeline
+// authors to declare that a step's failure is a client error (4xx), not a
+// server error (500), by setting error_status on the step config.
+type ErrorStatusStep struct {
+	inner  interfaces.PipelineStep
+	status int
+}
+
+// NewErrorStatusStep wraps inner so that any error it returns is wrapped in a
+// ValidationError with the given HTTP status code.
+func NewErrorStatusStep(inner interfaces.PipelineStep, status int) *ErrorStatusStep {
+	return &ErrorStatusStep{inner: inner, status: status}
+}
+
+// Name delegates to the wrapped step.
+func (s *ErrorStatusStep) Name() string {
+	return s.inner.Name()
+}
+
+// Execute runs the wrapped step. If it returns an error that is not already a
+// ValidationError, the error is wrapped in one with the configured status code.
+func (s *ErrorStatusStep) Execute(ctx context.Context, pc *PipelineContext) (*interfaces.StepResult, error) {
+	result, err := s.inner.Execute(ctx, pc)
+	if err != nil {
+		if !interfaces.IsValidationError(err) {
+			return nil, interfaces.NewValidationError(err.Error(), s.status)
+		}
+		return nil, err
+	}
+	return result, nil
+}

module/pipeline_step_error_status_test.go

@@ -0,0 +1,119 @@
+package module
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/GoCodeAlone/workflow/interfaces"
+)
+
+// fixedErrorStep is a test step that always returns a configured error.
+type fixedErrorStep struct {
+	name string
+	err  error
+}
+
+func (s *fixedErrorStep) Name() string { return s.name }
+func (s *fixedErrorStep) Execute(_ context.Context, _ *PipelineContext) (*interfaces.StepResult, error) {
+	return nil, s.err
+}
+
+// fixedSuccessStep is a test step that always succeeds.
+type fixedSuccessStep struct {
+	name string
+}
+
+func (s *fixedSuccessStep) Name() string { return s.name }
+func (s *fixedSuccessStep) Execute(_ context.Context, _ *PipelineContext) (*interfaces.StepResult, error) {
+	return &interfaces.StepResult{Output: map[string]any{"ok": true}}, nil
+}
+
+// TestErrorStatusStep_WrapsErrorWithValidationError verifies that a plain error
+// returned by the inner step is wrapped in a ValidationError with the configured
+// HTTP status code.
+func TestErrorStatusStep_WrapsErrorWithValidationError(t *testing.T) {
+	inner := &fixedErrorStep{name: "validate", err: errors.New("card doesn't match")}
+	step := NewErrorStatusStep(inner, 400)
+
+	_, err := step.Execute(context.Background(), &PipelineContext{
+		Current:  map[string]any{},
+		Metadata: map[string]any{},
+	})
+	if err == nil {
+		t.Fatal("expected error, got nil")
+	}
+	if !interfaces.IsValidationError(err) {
+		t.Fatalf("expected ValidationError, got %T: %v", err, err)
+	}
+	if got := interfaces.ValidationErrorStatus(err); got != 400 {
+		t.Errorf("expected status 400, got %d", got)
+	}
+	if err.Error() != "card doesn't match" {
+		t.Errorf("unexpected error message: %q", err.Error())
+	}
+}
+
+// TestErrorStatusStep_PassesThroughExistingValidationError verifies that a
+// ValidationError already set by the inner step is not double-wrapped.
+func TestErrorStatusStep_PassesThroughExistingValidationError(t *testing.T) {
+	inner := &fixedErrorStep{
+		name: "validate",
+		err:  interfaces.NewValidationError("already a validation error", 422),
+	}
+	step := NewErrorStatusStep(inner, 400)
+
+	_, err := step.Execute(context.Background(), &PipelineContext{
+		Current:  map[string]any{},
+		Metadata: map[string]any{},
+	})
+	if err == nil {
+		t.Fatal("expected error, got nil")
+	}
+	// Should keep the original 422, not override with 400
+	if got := interfaces.ValidationErrorStatus(err); got != 422 {
+		t.Errorf("expected original status 422 preserved, got %d", got)
+	}
+}
+
+// TestErrorStatusStep_SuccessPassesThrough verifies that a successful step
+// result is returned unchanged.
+func TestErrorStatusStep_SuccessPassesThrough(t *testing.T) {
+	inner := &fixedSuccessStep{name: "ok-step"}
+	step := NewErrorStatusStep(inner, 400)
+
+	result, err := step.Execute(context.Background(), &PipelineContext{
+		Current:  map[string]any{},
+		Metadata: map[string]any{},
+	})
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if result == nil || result.Output["ok"] != true {
+		t.Errorf("expected successful result with ok=true, got %v", result)
+	}
+}
+
+// TestStepConfig_ErrorStatus verifies that a pipeline with error_status on a
+// step surfaces the configured 4xx status when the step fails.
+func TestStepConfig_ErrorStatus(t *testing.T) {
+	inner := &fixedErrorStep{name: "validate", err: errors.New("invalid input")}
+	step := NewErrorStatusStep(inner, 400)
+
+	p := &Pipeline{
+		Name:    "test-pipeline",
+		Steps:   []PipelineStep{step},
+		OnError: ErrorStrategyStop,
+	}
+
+	_, err := p.Execute(context.Background(), map[string]any{})
+	if err == nil {
+		t.Fatal("expected error from pipeline")
+	}
+	if !interfaces.IsValidationError(err) {
+		t.Fatalf("expected ValidationError in pipeline error chain, got %T: %v", err, err)
+	}
+	if got := interfaces.ValidationErrorStatus(err); got != 400 {
+		t.Errorf("expected status 400 from pipeline error, got %d", got)
+	}
+}