OpenFn / OpenFn/lightning

WorkflowsController can crash on :workflow_deleted / :snapshot_failed

Open Beginner friendly
#4,960 0 comments 0 reactions 0 assignees View on GitHub

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 of save_workflow's failure reasons a proper
name (:snapshot_failed, previously a bare false), 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/:id fetches 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

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.