Stream file uploads, taking the data as a reader

[alvarolivie]

Blocked on invopop/api#126. Releasing this before that route exists 404s every file upload, not just large ones.

Sending a file's bytes inline means the API and the silo each hold the whole payload in memory, and the request is capped by the 4MiB gRPC message limit between them. A 12.8MB invoice hit that ceiling.

Part of APP-873, alongside invopop/silo#251 and invopop/api#126.

What changed

Files().Create now streams the data instead of sending it inline — it derives the sha256, mime and size, registers the file with no bytes, then PUTs the raw contents to the file's /data path.

Breaking: CreateSiloFile.Data is now an io.Reader. Callers holding bytes change Data: data to Data: bytes.NewReader(data); the compiler flags every call site. See Data as a reader below.

No size threshold. An earlier version kept small files inline to save a round trip; it's gone. It meant two code paths, a behaviour cliff at an arbitrary byte count, and small files still held whole in memory by both services. One path is simpler, and the round trip is in-cluster against workflows already making several calls.

Create accepts a registration with no data, given sha256, mime and size — which the API has always documented but this client rejected outright, without even exposing the fields. UploadData is exported for callers that already hold an io.Reader.

Worth your attention

• It's now two requests rather than one. If the second fails you're left with a registered-but-unstored file — inherent to the two-step flow the API documents, and a retry heals it: re-registering the same hash returns the existing file, and the upload is skipped when the silo reports the content already stored.
• CreateSiloFile.Data is tagged json:"-", so the contents can never be marshalled into the registration body.

Testing

go test -race ./... and golangci-lint green. Tests cover the two-call protocol and its ordering, that the payload never appears in the registration call, that the data call sends raw bytes rather than base64, the content-type default, the already-stored skip, and that a retry after a failed upload sends the contents again with both seekable and non-seekable readers.

Data as a reader

Data takes the contents as a reader, so a file on disk no longer has to be read into a []byte first, which is the one thing streaming was meant to avoid.

f, err := c.Silo().Files().Create(ctx, &invopop.CreateSiloFile{
    EntryID: entryID,
    Name:    "invoice.xml",
    Data:    file, // *os.File, bytes.NewReader(data), ...
})

An earlier revision added a separate CreateStream action and kept Data []byte. One Create taking a reader is simpler than two near-identical actions, and we're pre-1.0, so this breaks the field instead.

Registration has to describe the file before the contents go anywhere, so a bare reader cannot just be passed through. Three cases:

• The reader is an io.ReadSeeker. It's always read from its start, measured in one pass when SHA256/Size aren't given, and wound back. Covers *os.File and *bytes.Reader, which is nearly every real use. No extra memory, no temp file.
• Not seekable, SHA256 and Size already set. Sent straight through, so a once-through stream works. This is the one case that can't be retried.
• Not seekable, undescribed. Held in memory, exactly as the []byte path did. The held copy replaces Data on the request.

Retries. Create updates the request with the generated ID, hash, size and MIME. Because a seekable reader is rewound, and a non-seekable one is replaced by its held copy, calling Create again with the same request after a failed upload re-registers the same file and sends the contents again.

MIME is detected from the first 3072 bytes of the measuring pass, the most mimetype looks at.

I deliberately did not spool the non-seekable case to a temp file, the way silo does. Silo owns its disk; a client library writing to $TMPDIR unasked would be a surprise in a read-only container or a function with a small /tmp. The seekable path already avoids buffering for the case that matters.

🤖 Generated with Claude Code

[Copilot]

Copilot review overview

🟡 Changes recommended

Preserve request data when uploads fail so retries can complete successfully.

Review effort: Lite
Findings: 1

Open (1)

• Preserve request data when upload fails · New

What changed in this PR

Updates Silo file uploads to register metadata first, then stream raw data, with support for reader-based uploads.

Changes:

• Adds two-step streaming uploads and CreateStream.
• Adds hashing, MIME detection, size calculation, and raw request support.
• Expands streaming tests and shared response helpers.

File	Description
invopop/​silo_entries_test.go	Uses the shared response helper.
invopop/​silo_attachments.go	Implements streaming registration and uploads.
invopop/​silo_attachments_test.go	Tests streaming behavior and validation.
invopop/​invopop.go	Adds raw request-body support.
invopop/​helpers_test.go	Refactors JSON response fixtures.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot review overview

🟡 Changes recommended

Moderate issues remain for oversized non-seekable data and negative file sizes.

Review effort: Lite
Findings: 1

Open (1)

• Check non-seekable payload size before int32 conversion · New

Resolved since last review (1)

• Preserve request data when upload fails

Previously missed (1)

In code that hasn't changed since last review

Reject negative file sizes during registration

invopop/​silo_attachments.go:285

The registration guard rejects only Size == 0, so a data-less request with a negative Size and non-empty SHA256/MIME is serialized and sent as invalid file metadata. Since Size represents a byte count, reject all non-positive values here (consistent with the existing zero-size rejection).

[samlown]

Nice. I've made a few changes that break the SDK, but I think its better to start using io.Reader instead of byte arrays by default. This should be easy to fix in most circumstances.