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

[flemzord]

Problem (H7 + H8 — HIGH)

• H8: most read/write handlers mapped every backend error to 500. Unknown ids (readInstance, readWorkflow, runWorkflow, postEvent, abortWorkflowInstance, readInstanceHistory) returned 500 instead of 404, contradicting the spec and creating alerting noise.
• H7: createTrigger / testTrigger returned 500 for a malformed JSON body, and createWorkflow returned 500 for config-validation failures (and paniced on a post-wait GetInstance and a json.Marshal).

Fix

• New api.WriteError(w, r, err) central mapper: sql.ErrNoRows + workflow not-found sentinels + Temporal NotFound ⇒ 404; workflow.ErrInvalidConfig ⇒ 400; otherwise 500. Used across the read/run/post/abort/history/test handlers (v1 + v2).
• createTrigger / testTrigger: malformed body ⇒ 400.
• createWorkflow: Create now wraps validation errors as workflow.ErrInvalidConfig (⇒ 400); both panics replaced with proper error responses.

Tests

TestGetInstanceNotFound (404) and TestCreateWorkflowValidationError (400).

Severity: HIGH.

Note: this is the v1/v2-duplicated layer (see L13 in the review). Edits internal/workflow/manager.go (Create + a new sentinel) and many handler files shared with other PRs in this series — independent, different regions; merges may need trivial resolution.

[coderabbitai]

Walkthrough

This PR refactors API error handling to intelligently classify backend errors and map them to appropriate HTTP responses. A new WriteError helper function replaces generic 500-error responses, distinguishing not-found errors (404), configuration validation errors (400), and server errors (500). Workflow manager configuration validation failures are wrapped with ErrInvalidConfig sentinel. All v1 and v2 handler error paths are updated to use the new helper, and several handlers are improved to treat JSON decode failures as client validation errors and eliminate panic paths.

Changes

API Error Handling Infrastructure and Refactoring

Layer / File(s)	Summary
Error classification and mapping foundation internal/api/errors.go, internal/workflow/manager.go	New WriteError function routes errors to 404/400/500 responses based on type. ErrInvalidConfig sentinel marks config validation failures as client errors. WorkflowManager.Create wraps validation errors with the sentinel.
API v1 and v2 handlers refactored to use WriteError internal/api/v1/handler_*.go, internal/api/v2/handler_*.go	All handlers (read, create, post, abort, run endpoints in both versions) replace generic error responses with intelligent WriteError classification. JSON decode errors are treated as validation failures (400). Panic paths are replaced with error responses.
Test coverage for error classification behavior internal/api/v2/handler_create_workflow_test.go, internal/api/v2/handler_read_instance_test.go	New tests verify validation errors return 400 and not-found errors return 404, confirming end-to-end error classification.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A rabbit hops through error logs,
Classifying faults in code's fog—
404, 400, 500 too,
Each mapped right, each response true. ✨
No more panics in the night,
Just WriteError setting things right.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 4.55% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description comprehensively covers the changes across all modified files, clearly explaining the problems (H7/H8), the fixes implemented, and the test additions.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely summarizes the main change: mapping backend errors to appropriate HTTP status codes (400/404) instead of defaulting to 500, directly supported by the file summaries showing error-handling refactors across multiple handlers.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/api-error-mapping

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

internal/api/errors.go (1)

13-31: ⚡ Quick win

Add direct branch-level tests for the error mapper.

This helper now defines core API status semantics. Please add table-driven unit tests that exercise each branch (sql.ErrNoRows, workflow.ErrInstanceNotFound, workflow.ErrWorkflowNotFound, Temporal NotFound, workflow.ErrInvalidConfig, and default 500) to prevent silent regressions across handlers.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/api/errors.go` around lines 13 - 31, Add table-driven unit tests for
WriteError that exercise each branch: provide inputs of sql.ErrNoRows,
workflow.ErrInstanceNotFound, workflow.ErrWorkflowNotFound, a Temporal NotFound
(serviceerror.NotFound{}), workflow.ErrInvalidConfig, and a generic error for
the default case. For each case call WriteError with an
httptest.ResponseRecorder and a dummy *http.Request and assert the resulting
HTTP status and/or body matches the expected handler (sharedapi.NotFound -> 404,
sharedapi.BadRequest -> 400 with "VALIDATION" code,
sharedapi.InternalServerError -> 500). Name the test function
TestWriteError_TableDriven and place cases in a slice with descriptive names so
any failure shows which branch regressed; use the same identifiers WriteError,
workflow.ErrInstanceNotFound, workflow.ErrWorkflowNotFound,
workflow.ErrInvalidConfig, sql.ErrNoRows, and serviceerror.NotFound to locate
the logic under test.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@internal/api/errors.go`:
- Around line 13-31: Add table-driven unit tests for WriteError that exercise
each branch: provide inputs of sql.ErrNoRows, workflow.ErrInstanceNotFound,
workflow.ErrWorkflowNotFound, a Temporal NotFound (serviceerror.NotFound{}),
workflow.ErrInvalidConfig, and a generic error for the default case. For each
case call WriteError with an httptest.ResponseRecorder and a dummy *http.Request
and assert the resulting HTTP status and/or body matches the expected handler
(sharedapi.NotFound -> 404, sharedapi.BadRequest -> 400 with "VALIDATION" code,
sharedapi.InternalServerError -> 500). Name the test function
TestWriteError_TableDriven and place cases in a slice with descriptive names so
any failure shows which branch regressed; use the same identifiers WriteError,
workflow.ErrInstanceNotFound, workflow.ErrWorkflowNotFound,
workflow.ErrInvalidConfig, sql.ErrNoRows, and serviceerror.NotFound to locate
the logic under test.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 18477be9-0d2f-46e7-93a8-fa7089f405d8

📥 Commits

Reviewing files that changed from the base of the PR and between 271bf8d and 8b00f5a.

📒 Files selected for processing (21)

• internal/api/errors.go
• internal/api/v1/handler_abort_workflow_instance.go
• internal/api/v1/handler_create_trigger.go
• internal/api/v1/handler_create_workflow.go
• internal/api/v1/handler_post_event.go
• internal/api/v1/handler_read_instance.go
• internal/api/v1/handler_read_instance_history.go
• internal/api/v1/handler_read_workflow.go
• internal/api/v1/handler_run_workflow.go
• internal/api/v2/handler_abort_workflow_instance.go
• internal/api/v2/handler_create_trigger.go
• internal/api/v2/handler_create_workflow.go
• internal/api/v2/handler_create_workflow_test.go
• internal/api/v2/handler_post_event.go
• internal/api/v2/handler_read_instance.go
• internal/api/v2/handler_read_instance_history.go
• internal/api/v2/handler_read_instance_test.go
• internal/api/v2/handler_read_workflow.go
• internal/api/v2/handler_run_workflow.go
• internal/api/v2/handler_test_trigger.go
• internal/workflow/manager.go

[flemzord]

Tracked in Jira: EN-1227 (Epic EN-1217).

[flemzord]

Superseded by #199, which consolidates this change with the related reliability and safety fixes on top of the current main branch.