WorkflowsController can crash on :workflow_deleted / :snapshot_failed
Nobody has claimed this yet.
- Dominant language
- Elixir
- Stars
- 296
- Forks
- 86
- Avg merge
- 1d 13h
- Merged PRs (30d)
- 50
Description
Found while working on the AI-first starting UX epic (#4848), specifically
PR #4918. That PR gave one ofsave_workflow's failure reasons a proper
name (:snapshot_failed, previously a barefalse), which is what
surfaced this gap — the API controller was already missing a case for it
under the old name, it was just unreadable. No code from that epic needs
to change for this fix; this is a separate, pre-existing bug in a file
that epic doesn't touch.
What's wrong
Lightning.Workflows.save_workflow/3 can fail in a few different ways. Two of
them are: the workflow was deleted, or the workflow saved but its snapshot
(a frozen copy used for run history) failed to save.
The REST API controller for workflows
(lib/lightning_web/controllers/api/workflows_controller.ex) has a function,
maybe_handle_error/3, whose job is to turn a failure reason into a proper
HTTP response (like "422: your data is invalid" or "404: not found"). It
checks for a specific list of known failure reasons. Anything not on that
list gets returned unchanged.
The problem: a controller function in Phoenix (our web framework) is required
to return a special "response" value. If it returns something else instead —
like the raw error reason — the server crashes instead of sending back a
normal error message.
Neither "workflow was deleted" nor "snapshot failed" are on that checked
list, so if either happens through the API, the request crashes instead of
returning a clean error.
How to hit it
- Deleted workflow:
PATCH /api/workflows/:idfetches the workflow
first, but that fetch doesn't exclude deleted workflows. If the workflow
was soft-deleted in between, the save fails with "deleted" and the crash
happens. This bug already existed before PR #4918 — it's not new. - Snapshot failed: rarer — needs the snapshot to fail validation right
after the workflow itself saved successfully. Possible on both create and
update.
Suggested fix
Add two more cases to maybe_handle_error/3 for these two failure reasons,
returning a sensible error response (e.g. 404/409 for deleted, 500 for
snapshot failed) instead of falling through unchanged.
Context
Found while reviewing PR #4918, which gave the "snapshot failed" case a
proper name (:snapshot_failed) instead of returning a bare false. That
review is what surfaced this gap in the API controller — a different part
of the app (the collaborative editor) already had the same gap and got fixed
in that PR.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in lib/lightning_web/controllers/api/workflows_controller.ex at maybe_handle_error/3 and review the existing failure-reason clauses. Trace the PATCH, create, and update workflow paths for :workflow_deleted and :snapshot_failed, then verify each returns an HTTP response rather than the raw failure reason, including the deleted-workflow and snapshot-validation cases.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- elixir
- Domain
- api, backend
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 76/100