diff --git a/internal/api/errors.go b/internal/api/errors.go new file mode 100644 index 0000000..2c2ae4b --- /dev/null +++ b/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) + } +} diff --git a/internal/api/v1/handler_abort_workflow_instance.go b/internal/api/v1/handler_abort_workflow_instance.go index ea151fe..3ec4736 100644 --- a/internal/api/v1/handler_abort_workflow_instance.go +++ b/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) diff --git a/internal/api/v1/handler_create_trigger.go b/internal/api/v1/handler_create_trigger.go index 359f93a..f94e678 100644 --- a/internal/api/v1/handler_create_trigger.go +++ b/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 } diff --git a/internal/api/v1/handler_create_workflow.go b/internal/api/v1/handler_create_workflow.go index 926f637..f8c3cfa 100644 --- a/internal/api/v1/handler_create_workflow.go +++ b/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 } diff --git a/internal/api/v1/handler_post_event.go b/internal/api/v1/handler_post_event.go index 5d5ce64..269969a 100644 --- a/internal/api/v1/handler_post_event.go +++ b/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 } diff --git a/internal/api/v1/handler_read_instance.go b/internal/api/v1/handler_read_instance.go index d0b94a8..de51583 100644 --- a/internal/api/v1/handler_read_instance.go +++ b/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 } diff --git a/internal/api/v1/handler_read_instance_history.go b/internal/api/v1/handler_read_instance_history.go index 4fc6de9..33dfe4e 100644 --- a/internal/api/v1/handler_read_instance_history.go +++ b/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 } diff --git a/internal/api/v1/handler_read_workflow.go b/internal/api/v1/handler_read_workflow.go index e00432c..0eb8e6b 100644 --- a/internal/api/v1/handler_read_workflow.go +++ b/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 } diff --git a/internal/api/v1/handler_run_workflow.go b/internal/api/v1/handler_run_workflow.go index a0ac1a2..7e03c38 100644 --- a/internal/api/v1/handler_run_workflow.go +++ b/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) diff --git a/internal/api/v2/handler_abort_workflow_instance.go b/internal/api/v2/handler_abort_workflow_instance.go index 4d16f8e..578b695 100644 --- a/internal/api/v2/handler_abort_workflow_instance.go +++ b/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) diff --git a/internal/api/v2/handler_create_trigger.go b/internal/api/v2/handler_create_trigger.go index 8ca5c20..20d5218 100644 --- a/internal/api/v2/handler_create_trigger.go +++ b/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 } diff --git a/internal/api/v2/handler_create_workflow.go b/internal/api/v2/handler_create_workflow.go index 0fe7292..41e5c08 100644 --- a/internal/api/v2/handler_create_workflow.go +++ b/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 } diff --git a/internal/api/v2/handler_create_workflow_test.go b/internal/api/v2/handler_create_workflow_test.go index 7a80f21..1ec0c3b 100644 --- a/internal/api/v2/handler_create_workflow_test.go +++ b/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) + }) +} diff --git a/internal/api/v2/handler_post_event.go b/internal/api/v2/handler_post_event.go index 2f6afea..2d1c509 100644 --- a/internal/api/v2/handler_post_event.go +++ b/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 } diff --git a/internal/api/v2/handler_read_instance.go b/internal/api/v2/handler_read_instance.go index 2ac07d4..889e897 100644 --- a/internal/api/v2/handler_read_instance.go +++ b/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 } diff --git a/internal/api/v2/handler_read_instance_history.go b/internal/api/v2/handler_read_instance_history.go index 11a25be..5114703 100644 --- a/internal/api/v2/handler_read_instance_history.go +++ b/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 } diff --git a/internal/api/v2/handler_read_instance_test.go b/internal/api/v2/handler_read_instance_test.go index a881b31..9a99416 100644 --- a/internal/api/v2/handler_read_instance_test.go +++ b/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) + }) +} diff --git a/internal/api/v2/handler_read_workflow.go b/internal/api/v2/handler_read_workflow.go index 024e772..b7970cd 100644 --- a/internal/api/v2/handler_read_workflow.go +++ b/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 } diff --git a/internal/api/v2/handler_run_workflow.go b/internal/api/v2/handler_run_workflow.go index 8bc56ab..2cf758b 100644 --- a/internal/api/v2/handler_run_workflow.go +++ b/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) diff --git a/internal/api/v2/handler_test_trigger.go b/internal/api/v2/handler_test_trigger.go index 152537e..b268fd2 100644 --- a/internal/api/v2/handler_test_trigger.go +++ b/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 } diff --git a/internal/workflow/manager.go b/internal/workflow/manager.go index 8460c61..44c7a30 100644 --- a/internal/workflow/manager.go +++ b/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)