OpenFn / OpenFn/lightning

History export has a few correctness and reliability bugs we never caught

Open
#4,777 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

What's broken

There are four problems in the history export, in rough order of impact.

The big one is that the produced export.json is invalid JSON for any export that crosses the batch threshold (currently 50 work orders). The worker encodes each batch's payload separately and writes it to the same file with [:append], so a multi-batch export produces several concatenated top-level objects rather than one document. Any consumer trying to decode the file will fail on the second object.

Every exported step is also missing its snapshot_id. The formatter calls Map.take/2 with the atom :snapshot_it (typo) instead of :snapshot_id, and Map.take/2 silently drops unknown keys, so the field is just absent from the output.

If an export fails after it has been enqueued, the user gets no notification. The Projects.File row is marked :failed and the error is logged, but the user notifier only fires on the success path. From the user's point of view, they were told in the confirmation modal that they would receive an email when the export was complete, and then nothing arrives.

Finally, the confirm-export handler in the run history LiveView doesn't go through a permissions policy. Every other mutating handler in that LiveView guards through Permissions.can/4, but this one calls Invocation.export_workorders/3 directly. Download is properly authorized via Lightning.Policies.Exports, so data isn't escaping, but any project member regardless of role can enqueue an arbitrarily large export.

How it surfaced

I walked through the history export flow on staging and then read the exporter code to verify what I was seeing. None of these are covered by the existing worker tests, which only exercise a single work order through the happy path and never produce more than one batch.

What to fix

  • Stop writing export.json per batch. Either accumulate the entity payload and write once at the end of the stream, or write per-batch into separate files and merge as part of finalisation. Add a test that exports more than @batch_size work orders and asserts the resulting export.json decodes cleanly.
  • Correct the :snapshot_it typo in the step formatter and add a regression test asserting snapshot_id is present in the exported step rows.
  • Send a failure notification when the worker errors out, so the user isn't left waiting on a success email that never arrives. Email parity with the success path is probably the cleanest, with the failure reason kept generic.
  • Route confirm-export through the same Permissions.can/4 pattern used by the rest of the mutating handlers in the run history LiveView.

A few smaller things worth doing while we're in there:

  • Resolve or delete the three # TODO comments in the worker, one of which is literally "let's chat about this".
  • Make extract_entities/1 and fetch_dataclips/1 private; nothing outside the worker calls them.
  • Filter nil dataclip ids out of the dataclip extraction step before hitting the database.
  • Reconsider Jason.encode!(..., pretty: true). Exports aren't read by humans and the extra bytes add up on large projects.

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 with the history export worker and its existing worker tests, then inspect the run history LiveView's confirm-export handler and the Permissions.can/4 pattern used by other mutating handlers. Verify that multi-batch export.json files decode, exported steps include snapshot_id, failures notify users, and confirmation enforces the appropriate permission; treat the listed cleanup items as additional scope.

Written by the indexing model from the issue text.

Assessment

Tech stack
elixir
Domain
authorization, backend, testing
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.