From 2e322d4fb6541d789852c997512158104e62a6a1 Mon Sep 17 00:00:00 2001 From: Asish Kumar Date: Sat, 4 Apr 2026 14:51:07 +0530 Subject: [PATCH 1/3] fix: mark project file as failed when export worker errors ExportWorker.perform/1 set the ProjectFile status to :in_progress at the start of the export but never updated it to :failed when the with chain encountered an error. With max_attempts: 1, this left the record permanently orphaned with status :in_progress and path nil. The data retention cron then crashed when iterating over these orphaned records because it passed nil to Lightning.Storage.delete/1, generating ~56,000 Sentry events. Fix both problems: - Restructure the with/else block in perform/1 to call a new mark_project_file_failed/1 helper on any error, setting the ProjectFile status to :failed before returning the error to Oban. - Add a nil-path guard clause in remove_expired_files_for/1 so that orphaned files are logged and deleted from the database without attempting storage deletion. Closes #4454 Signed-off-by: Asish Kumar --- CHANGELOG.md | 6 +++ lib/lightning/projects.ex | 19 +++++--- lib/lightning/workorders/export_worker.ex | 53 +++++++++++++++-------- test/lightning/export_worker_test.exs | 22 ++++++++++ test/lightning/projects_test.exs | 22 ++++++++++ 5 files changed, 98 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d4bc3c4903..64d578818e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,12 @@ and this project adheres to ### Fixed +- ExportWorker now marks the ProjectFile as `:failed` when the export process + errors, preventing records from being stuck permanently as `:in_progress` with + a nil path. The data retention cron also handles orphaned files with nil paths + gracefully instead of crashing. + [#4454](https://github.com/OpenFn/lightning/issues/4454) + ## [2.16.1-pre1] - 2026-04-04 ### Added diff --git a/lib/lightning/projects.ex b/lib/lightning/projects.ex index 32d663ac7e5..1cfd22ce72f 100644 --- a/lib/lightning/projects.ex +++ b/lib/lightning/projects.ex @@ -1021,13 +1021,22 @@ defmodule Lightning.Projects do f.project_id == ^project_id and f.inserted_at < ago(^period, "day") ) |> Repo.all() - |> Enum.each(fn %{path: object_path} = project_file -> - result = Lightning.Storage.delete(object_path) + |> Enum.each(fn + %{path: nil} = project_file -> + Logger.warning( + "Deleting orphaned project file #{project_file.id} " <> + "with nil path (likely a failed export)" + ) - if match?({:ok, _res}, result) or - match?({:error, %{status: 404}}, result) do Repo.delete(project_file) - end + + %{path: object_path} = project_file -> + result = Lightning.Storage.delete(object_path) + + if match?({:ok, _res}, result) or + match?({:error, %{status: 404}}, result) do + Repo.delete(project_file) + end end) end diff --git a/lib/lightning/workorders/export_worker.ex b/lib/lightning/workorders/export_worker.ex index a4da7d8dc74..4fc006b567d 100644 --- a/lib/lightning/workorders/export_worker.ex +++ b/lib/lightning/workorders/export_worker.ex @@ -53,32 +53,35 @@ defmodule Lightning.WorkOrders.ExportWorker do }) do search_params = SearchParams.from_map(params) - result = - with {:ok, project_file} <- get_project_file(project_file_id), - {:ok, project_file} <- - update_project_file(project_file, %{status: :in_progress}), - {:ok, project} <- get_project(project_id), - {:ok, zip_file} <- - process_export(project, search_params, project_file), - {:ok, storage_path} <- store_project_file(zip_file, project_file) do + with {:ok, project_file} <- get_project_file(project_file_id), + {:ok, project_file} <- + update_project_file(project_file, %{status: :in_progress}), + {:ok, project} <- get_project(project_id), + {:ok, zip_file} <- + process_export(project, search_params, project_file), + {:ok, storage_path} <- + store_project_file(zip_file, project_file) do + {:ok, project_file} = update_project_file(project_file, %{ status: :completed, path: storage_path }) - end - - case result do - {:ok, project_file} -> - UserNotifier.notify_history_export_completion( - project_file.created_by, - project_file - ) - Logger.info("Export completed successfully.") - :ok + UserNotifier.notify_history_export_completion( + project_file.created_by, + project_file + ) + Logger.info("Export completed successfully.") + :ok + else {:error, reason} -> - Logger.error("Export failed with reason: #{inspect(reason)}") + mark_project_file_failed(project_file_id) + + Logger.error( + "Export failed with reason: #{inspect(reason)}" + ) + {:error, reason} end end @@ -499,4 +502,16 @@ defmodule Lightning.WorkOrders.ExportWorker do %{id: log_line.id, message: log_line.message, run_id: log_line.run_id} end) end + + defp mark_project_file_failed(project_file_id) do + case Repo.get(Projects.File, project_file_id) do + nil -> + :ok + + project_file -> + project_file + |> Projects.File.mark_failed() + |> Repo.update() + end + end end diff --git a/test/lightning/export_worker_test.exs b/test/lightning/export_worker_test.exs index 0b1f5bfdec3..f3a36ff3469 100644 --- a/test/lightning/export_worker_test.exs +++ b/test/lightning/export_worker_test.exs @@ -183,6 +183,28 @@ defmodule Lightning.ExportWorkerTest do :zip.zip_close(zip_handle) end + + test "marks project file as failed when export fails", + %{ + project_file: project_file, + search_params: search_params + } do + non_existent_project_id = Ecto.UUID.generate() + + assert {:error, :project_not_found} == + ExportWorker.perform(%Oban.Job{ + args: %{ + "project_id" => non_existent_project_id, + "project_file" => project_file.id, + "search_params" => + to_string_key_map(search_params) + } + }) + + project_file = Repo.reload(project_file) + assert project_file.status == :failed + assert is_nil(project_file.path) + end end def extract_and_read(zip_file_path, target_file_name) do diff --git a/test/lightning/projects_test.exs b/test/lightning/projects_test.exs index 19577d1e40e..23297733ae3 100644 --- a/test/lightning/projects_test.exs +++ b/test/lightning/projects_test.exs @@ -1500,6 +1500,28 @@ defmodule Lightning.ProjectsTest do assert Repo.get(Projects.File, project_file2.id) end + test "deletes orphaned project files with nil path" do + project = + insert(:project, history_retention_period: 7) + + more_days_ago = Date.utc_today() |> Date.add(-8) + + orphaned_file = + insert(:project_file, + project: project, + path: nil, + status: :in_progress, + inserted_at: DateTime.new!(more_days_ago, ~T[00:00:00]) + ) + + :ok = + Projects.perform(%Oban.Job{ + args: %{"type" => "data_retention"} + }) + + refute Repo.get(Projects.File, orphaned_file.id) + end + test "deletes channel request history based on started_at" do project = insert(:project, history_retention_period: 7) channel = insert(:channel, project: project) From 3b2a82201631f9d8d530bd8822d8f7b15a37136f Mon Sep 17 00:00:00 2001 From: Asish Kumar Date: Sat, 4 Apr 2026 15:11:00 +0530 Subject: [PATCH 2/3] style: fix formatting in export_worker and test Collapse single-line expressions to satisfy mix format --check-formatted. Signed-off-by: Asish Kumar --- lib/lightning/workorders/export_worker.ex | 4 +--- test/lightning/export_worker_test.exs | 3 +-- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/lightning/workorders/export_worker.ex b/lib/lightning/workorders/export_worker.ex index 4fc006b567d..1c86849d6ea 100644 --- a/lib/lightning/workorders/export_worker.ex +++ b/lib/lightning/workorders/export_worker.ex @@ -78,9 +78,7 @@ defmodule Lightning.WorkOrders.ExportWorker do {:error, reason} -> mark_project_file_failed(project_file_id) - Logger.error( - "Export failed with reason: #{inspect(reason)}" - ) + Logger.error("Export failed with reason: #{inspect(reason)}") {:error, reason} end diff --git a/test/lightning/export_worker_test.exs b/test/lightning/export_worker_test.exs index f3a36ff3469..2682e52e1ee 100644 --- a/test/lightning/export_worker_test.exs +++ b/test/lightning/export_worker_test.exs @@ -196,8 +196,7 @@ defmodule Lightning.ExportWorkerTest do args: %{ "project_id" => non_existent_project_id, "project_file" => project_file.id, - "search_params" => - to_string_key_map(search_params) + "search_params" => to_string_key_map(search_params) } }) From 308f58b92e194e076e58bd7faa4e583d6d229626 Mon Sep 17 00:00:00 2001 From: Asish Kumar Date: Sat, 4 Apr 2026 15:35:14 +0530 Subject: [PATCH 3/3] fix: handle update failures in export worker error paths Moved the project file update into the with chain to prevent MatchError when the update fails. Added error logging when mark_project_file_failed encounters a repo update failure. Signed-off-by: Asish Kumar --- lib/lightning/workorders/export_worker.ex | 24 ++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/lib/lightning/workorders/export_worker.ex b/lib/lightning/workorders/export_worker.ex index 1c86849d6ea..1b9914d74cf 100644 --- a/lib/lightning/workorders/export_worker.ex +++ b/lib/lightning/workorders/export_worker.ex @@ -60,13 +60,12 @@ defmodule Lightning.WorkOrders.ExportWorker do {:ok, zip_file} <- process_export(project, search_params, project_file), {:ok, storage_path} <- - store_project_file(zip_file, project_file) do - {:ok, project_file} = - update_project_file(project_file, %{ - status: :completed, - path: storage_path - }) - + store_project_file(zip_file, project_file), + {:ok, project_file} <- + update_project_file(project_file, %{ + status: :completed, + path: storage_path + }) do UserNotifier.notify_history_export_completion( project_file.created_by, project_file @@ -510,6 +509,17 @@ defmodule Lightning.WorkOrders.ExportWorker do project_file |> Projects.File.mark_failed() |> Repo.update() + |> case do + {:ok, _project_file} -> + :ok + + {:error, changeset} -> + Logger.error( + "Failed to mark project file #{project_file_id} as failed: #{inspect(changeset.errors)}" + ) + + {:error, changeset} + end end end end