Skip to content

File uploads cannot stream: StorableFile::content() forces the whole file into memory #944

Description

@wadakatu

Description

Every file upload path in the SDK reads the complete file into a PHP string before sending it, because the storable-file contract only offers a string:

// src/Contracts/Files/HasContent.php
interface HasContent extends Stringable
{
    public function content(): string;
}

StorableFile extends HasContent, so LocalDocument does file_get_contents(), StoredDocument reads the whole disk object, Base64Document holds base64, and PreparesStorableFiles hands the resulting string to the gateway:

// src/Gateway/Concerns/PreparesStorableFiles.php
return [
    $file->content(),
    $file->mimeType() ?? 'application/octet-stream',
    $file->name() ?? 'file',
];

Why this is worth changing

The transport is already capable of streaming. Http::attach() accepts string|resource, and Guzzle turns whatever it receives into a stream (MultipartStream::addElement()Utils::streamFor()). The only thing forcing the buffer is the content(): string return type.

For a provider whose documented per-file limit is 512 MB (OpenAI Files), the SDK currently cannot upload anything the worker's memory_limit will not hold in full. On a shared FPM pool a single large upload can also starve unrelated requests.

Suggested direction

The SDK already has a pattern for optional per-file capabilities — HasProviderOptions, checked with instanceof in the same trait:

// src/Gateway/Concerns/PreparesStorableFiles.php
return $file instanceof HasProviderOptions ? $file->providerOptions($provider) : [];

A streaming capability could follow the same shape, keeping HasContent untouched and every existing implementation working:

interface HasStream
{
    /** @return resource */
    public function stream();
}

// PreparesStorableFiles
$contents = $file instanceof HasStream ? $file->stream() : $file->content();

LocalDocument would return fopen($this->path, 'rb') and StoredDocument would return Storage::disk($disk)->readStream($path). Base64Document would not implement it — it genuinely holds its content in memory already. No change is needed at the Http::attach() call site.

Two design questions I do not want to assume the answer to

  1. Stream ownership. Whoever opens the handle has to close it. Either the gateway wraps the send in try/finally, or the file object owns the handle and closes it on destruct. The first is simpler but means the gateway closes something it did not open.

  2. Failover and retries. A stream that has been read cannot be re-sent. Seekable handles can be rewound, but a non-seekable one (a remote or pipe-backed stream) cannot, so provider failover would need either a "streaming only when seekable" rule or a way to reopen the source. This is the part that seems most likely to shape the API, which is why I have described the direction rather than proposing a finished design.

Related

#943 covers a smaller, independent problem on the same path: Files::put() base64-encodes an UploadedFile that already has a real path, costing 2.67× the file size in peak memory where reading the path costs 1.00×. That one is a one-line mapping change and needs no contract work; this issue is about the remaining 1.00× and the contract that causes it.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions