Skip to content

fix: stream PayloadFile uploads end-to-end (AD0007)#62

Merged
daniellemccool merged 3 commits into
masterfrom
fix/streaming-payloadfile
May 4, 2026
Merged

fix: stream PayloadFile uploads end-to-end (AD0007)#62
daniellemccool merged 3 commits into
masterfrom
fix/streaming-payloadfile

Conversation

@daniellemccool
Copy link
Copy Markdown

Closes regression introduced in 68c59d8. Restores the streaming intent of upstream PR eyra#482.

Production reproduction confirmed against next.eyra.co with a 2.5 GiB Facebook fixture (FileReaderSync NotReadableError when materialize_file calls adapter.read() with no size).

PR 1 of 3:

  • PR 1 (this): the fix + AD0007 + tests
  • PR 2: type tightening + dead PayloadString JS cleanup + parameter renames (zip_path → archive)
  • PR 3: project CLAUDE.md + doc drift fixes

Fixes #61

…on/AD0007

      documenting the streaming invariant: PayloadFile\nuploads must be passed
      to consumers (zipfile.ZipFile, validators,\nextractors) without
      materializing to a path. The previous\nmaterialize_file() helper called
      AsyncFileAdapter.read() with no size\nargument, triggering
      FileReaderSync.readAsArrayBuffer() against the\nentire blob, which fails
      with NotReadableError above the DOM File\nAPI ~2 GiB ArrayBuffer
      cap.\n\nSucceeds extraction/AD0003 (whose ownership decision —
      FlowBuilder\nowns upload safety — remains correct; only the size-check
      placement\nchanges).\n\nReferences eyra/feldspar PR eyra#482 for upstream
      design intent and\nfeldspar/AD0003 for the PayloadFile migration whose
      intent this ADR\nenforces. Closes the deprecation window for
      PayloadString/WORKERFS\n(SRC compat dropped 2026-04-30).\n\nRefs:
      #61\n')
…accepts PayloadFile, adds logging for Upload skipped and other diagnostic logs (Upload prompt sent, Upload received).
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the ADRs in general. But I think they are a bit too verbose.

I would like them as short as possible.

Copy link
Copy Markdown
Collaborator

@trbKnl trbKnl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick fix.

@trbKnl
Copy link
Copy Markdown
Collaborator

trbKnl commented May 1, 2026

I see the PR is directly on master, which is fine, but we go do:

  • merge it now on master
  • then make sure dev is up to date
  • do the other PR's on dev
  • merge master with semantic versioning

…orcing with CI, and reduced the verbosity of the ADR
@daniellemccool daniellemccool merged commit de31dea into master May 4, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants