fix(api): map backend errors to 400/404 instead of 500 (EN-1227)

internal/api/errors.go

@@ -0,0 +1,32 @@
+package api
+
+import (
+	"database/sql"
+	"net/http"
+
+	sharedapi "github.com/formancehq/go-libs/v3/api"
+	"github.com/formancehq/orchestration/internal/workflow"
+	"github.com/pkg/errors"
+	"go.temporal.io/api/serviceerror"
+)
+
+// WriteError maps a backend error to the appropriate HTTP response:
+//   - 404 for not-found errors: sql.ErrNoRows, the workflow not-found
+//     sentinels, and Temporal NotFound (raised when reading the history of an
+//     unknown instance/stage);
+//   - 400 for invalid workflow configuration;
+//   - 500 otherwise.
+func WriteError(w http.ResponseWriter, r *http.Request, err error) {
+	var temporalNotFound *serviceerror.NotFound
+	switch {
+	case errors.As(err, &temporalNotFound),
+		errors.Is(err, sql.ErrNoRows),
+		errors.Is(err, workflow.ErrInstanceNotFound),
+		errors.Is(err, workflow.ErrWorkflowNotFound):
+		sharedapi.NotFound(w, err)
+	case errors.Is(err, workflow.ErrInvalidConfig):
+		sharedapi.BadRequest(w, "VALIDATION", err)
+	default:
+		sharedapi.InternalServerError(w, r, err)
+	}
+}

internal/api/v1/handler_abort_workflow_instance.go

@@ -11,7 +11,7 @@ import (
 func abortWorkflowInstance(backend api2.Backend) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		if err := backend.AbortRun(r.Context(), instanceID(r)); err != nil {
-			api.InternalServerError(w, r, err)
+			api2.WriteError(w, r, err)
 			return
 		}
 		api.NoContent(w)

internal/api/v1/handler_create_trigger.go

@@ -16,7 +16,7 @@ func createTrigger(backend api.Backend) func(writer http.ResponseWriter, request
 
 		data := triggers.TriggerData{}
 		if err := json.NewDecoder(r.Body).Decode(&data); err != nil {
-			sharedapi.InternalServerError(w, r, err)
+			sharedapi.BadRequest(w, "VALIDATION", err)
 			return
 		}
 

internal/api/v1/handler_create_workflow.go

@@ -25,7 +25,8 @@ func createWorkflow(m api2.Backend) http.HandlerFunc {
 
 			asJson, err := json.Marshal(payload)
 			if err != nil {
-				panic(err)
+				api.InternalServerError(w, r, err)
+				return
 			}
 
 			if err := json.Unmarshal(asJson, &config); err != nil {
@@ -41,7 +42,7 @@ func createWorkflow(m api2.Backend) http.HandlerFunc {
 
 		workflow, err := m.Create(r.Context(), config)
 		if err != nil {
-			api.InternalServerError(w, r, errors.Wrap(err, "creating workflow"))
+			api2.WriteError(w, r, errors.Wrap(err, "creating workflow"))
 			return
 		}
 

internal/api/v1/handler_post_event.go

@@ -19,7 +19,7 @@ func postEventToWorkflowInstance(backend api2.Backend) http.HandlerFunc {
 		}
 
 		if err := backend.PostEvent(r.Context(), instanceID(r), event); err != nil {
-			api.InternalServerError(w, r, err)
+			api2.WriteError(w, r, err)
 			return
 		}
 

internal/api/v1/handler_read_instance.go

@@ -12,7 +12,7 @@ func readInstance(backend api2.Backend) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		workflows, err := backend.GetInstance(r.Context(), instanceID(r))
 		if err != nil {
-			api.InternalServerError(w, r, err)
+			api2.WriteError(w, r, err)
 			return
 		}
 

internal/api/v1/handler_read_instance_history.go

@@ -12,7 +12,7 @@ func readInstanceHistory(backend api2.Backend) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		workflows, err := backend.ReadInstanceHistory(r.Context(), instanceID(r))
 		if err != nil {
-			api.InternalServerError(w, r, err)
+			api2.WriteError(w, r, err)
 			return
 		}
 

internal/api/v1/handler_read_workflow.go

@@ -12,7 +12,7 @@ func readWorkflow(backend api2.Backend) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		workflow, err := backend.ReadWorkflow(r.Context(), workflowID(r))
 		if err != nil {
-			api.InternalServerError(w, r, err)
+			api2.WriteError(w, r, err)
 			return
 		}
 

internal/api/v1/handler_run_workflow.go

@@ -22,7 +22,7 @@ func runWorkflow(backend api2.Backend) http.HandlerFunc {
 		}
 		instance, err := backend.RunWorkflow(r.Context(), workflowID(r), input)
 		if err != nil {
-			api.InternalServerError(w, r, err)
+			api2.WriteError(w, r, err)
 			return
 		}
 
@@ -38,7 +38,8 @@ func runWorkflow(backend api2.Backend) http.HandlerFunc {
 			}
 			ret.Instance, err = backend.GetInstance(r.Context(), instance.ID)
 			if err != nil {
-				panic(err)
+				api2.WriteError(w, r, err)
+				return
 			}
 
 			api.Created(w, ret)

internal/api/v2/handler_abort_workflow_instance.go

@@ -11,7 +11,7 @@ import (
 func abortWorkflowInstance(backend api2.Backend) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		if err := backend.AbortRun(r.Context(), instanceID(r)); err != nil {
-			api.InternalServerError(w, r, err)
+			api2.WriteError(w, r, err)
 			return
 		}
 		api.NoContent(w)

internal/api/v2/handler_create_trigger.go

@@ -16,7 +16,7 @@ func createTrigger(backend api.Backend) func(writer http.ResponseWriter, request
 
 		data := triggers.TriggerData{}
 		if err := json.NewDecoder(r.Body).Decode(&data); err != nil {
-			sharedapi.InternalServerError(w, r, err)
+			sharedapi.BadRequest(w, "VALIDATION", err)
 			return
 		}
 

internal/api/v2/handler_create_workflow.go

@@ -25,7 +25,8 @@ func createWorkflow(m api.Backend) http.HandlerFunc {
 
 			asJson, err := json.Marshal(payload)
 			if err != nil {
-				panic(err)
+				sharedapi.InternalServerError(w, r, err)
+				return
 			}
 
 			if err := json.Unmarshal(asJson, &config); err != nil {
@@ -41,7 +42,7 @@ func createWorkflow(m api.Backend) http.HandlerFunc {
 
 		workflow, err := m.Create(r.Context(), config)
 		if err != nil {
-			sharedapi.InternalServerError(w, r, errors.Wrap(err, "creating workflow"))
+			api.WriteError(w, r, errors.Wrap(err, "creating workflow"))
 			return
 		}
 

internal/api/v2/handler_create_workflow_test.go

@@ -23,3 +23,16 @@ func TestCreateWorkflow(t *testing.T) {
 		require.Equal(t, http.StatusCreated, rec.Result().StatusCode)
 	})
 }
+
+func TestCreateWorkflowValidationError(t *testing.T) {
+	test(t, func(router *chi.Mux, m api.Backend, db *bun.DB) {
+		// An empty stage specification fails config validation; the API must
+		// answer 400, not 500.
+		req := httptest.NewRequest(http.MethodPost, "/workflows", bytes.NewBufferString(`{"stages": [{}]}`))
+		rec := httptest.NewRecorder()
+
+		router.ServeHTTP(rec, req)
+
+		require.Equal(t, http.StatusBadRequest, rec.Result().StatusCode)
+	})
+}

internal/api/v2/handler_post_event.go

@@ -19,7 +19,7 @@ func postEventToWorkflowInstance(backend api2.Backend) http.HandlerFunc {
 		}
 
 		if err := backend.PostEvent(r.Context(), instanceID(r), event); err != nil {
-			api.InternalServerError(w, r, err)
+			api2.WriteError(w, r, err)
 			return
 		}
 

internal/api/v2/handler_read_instance.go

@@ -12,7 +12,7 @@ func readInstance(backend api.Backend) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		workflows, err := backend.GetInstance(r.Context(), instanceID(r))
 		if err != nil {
-			sharedapi.InternalServerError(w, r, err)
+			api.WriteError(w, r, err)
 			return
 		}
 

internal/api/v2/handler_read_instance_history.go

@@ -12,7 +12,7 @@ func readInstanceHistory(backend api2.Backend) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		workflows, err := backend.ReadInstanceHistory(r.Context(), instanceID(r))
 		if err != nil {
-			api.InternalServerError(w, r, err)
+			api2.WriteError(w, r, err)
 			return
 		}
 

internal/api/v2/handler_read_instance_test.go

@@ -52,3 +52,14 @@ func TestGetInstance(t *testing.T) {
 		require.Len(t, retrievedInstance.Statuses, 10)
 	})
 }
+
+func TestGetInstanceNotFound(t *testing.T) {
+	test(t, func(router *chi.Mux, m api.Backend, db *bun.DB) {
+		req := httptest.NewRequest(http.MethodGet, "/instances/does-not-exist", nil)
+		rec := httptest.NewRecorder()
+
+		router.ServeHTTP(rec, req)
+
+		require.Equal(t, http.StatusNotFound, rec.Result().StatusCode)
+	})
+}

internal/api/v2/handler_read_workflow.go

@@ -12,7 +12,7 @@ func readWorkflow(backend api2.Backend) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		workflow, err := backend.ReadWorkflow(r.Context(), workflowID(r))
 		if err != nil {
-			api.InternalServerError(w, r, err)
+			api2.WriteError(w, r, err)
 			return
 		}
 

internal/api/v2/handler_run_workflow.go

@@ -22,7 +22,7 @@ func runWorkflow(backend api2.Backend) http.HandlerFunc {
 		}
 		instance, err := backend.RunWorkflow(r.Context(), workflowID(r), input)
 		if err != nil {
-			api.InternalServerError(w, r, err)
+			api2.WriteError(w, r, err)
 			return
 		}
 
@@ -38,7 +38,8 @@ func runWorkflow(backend api2.Backend) http.HandlerFunc {
 			}
 			ret.Instance, err = backend.GetInstance(r.Context(), instance.ID)
 			if err != nil {
-				panic(err)
+				api2.WriteError(w, r, err)
+				return
 			}
 
 			api.Created(w, ret)

internal/api/v2/handler_test_trigger.go

@@ -15,13 +15,13 @@ func testTrigger(backend api.Backend) http.HandlerFunc {
 
 		data := make(map[string]any)
 		if err := json.NewDecoder(r.Body).Decode(&data); err != nil {
-			sharedapi.InternalServerError(w, r, err)
+			sharedapi.BadRequest(w, "VALIDATION", err)
 			return
 		}
 
 		o, err := backend.TestTrigger(r.Context(), chi.URLParam(r, "triggerID"), data)
 		if err != nil {
-			sharedapi.InternalServerError(w, r, err)
+			api.WriteError(w, r, err)
 			return
 		}
 

internal/workflow/manager.go

@@ -23,6 +23,9 @@ import (
 var (
 	ErrInstanceNotFound = errors.New("Instance not found")
 	ErrWorkflowNotFound = errors.New("Workflow not found")
+	// ErrInvalidConfig wraps workflow configuration validation failures so the
+	// API can map them to 400 instead of 500.
+	ErrInvalidConfig = errors.New("invalid workflow configuration")
 )
 
 const (
@@ -44,7 +47,7 @@ type WorkflowManager struct {
 func (m *WorkflowManager) Create(ctx context.Context, config Config) (*Workflow, error) {
 
 	if err := config.Validate(); err != nil {
-		return nil, err
+		return nil, fmt.Errorf("%w: %s", ErrInvalidConfig, err)
 	}
 
 	workflow := New(config)