fix(storage): close upload file handle when the request fails

[chengwudi1]

What

Fixes #1575 — when upload() / update() / upload_to_signed_url() fail, the file handle storage3 opened itself was
never closed: _request closed it after the except HTTPStatusError block, so any non-2xx raised
StorageApiError straight past it, leaking the handle (and emitting ResourceWarning: unclosed file).

How

Moved the close into a finally block so it runs on both success and error paths. The condition is unchanged — still
only BufferedReader handles (i.e. only handles storage3 opened itself), so a caller-supplied stream is left alone.

• _async/file_api.py: hand-written fix
• _sync/file_api.py: regenerated with unasync (from the _async source), no unrelated changes

Tests

• _sync/file_api.py: regenerated with unasync (from the _async source), no unrelated changes

Tests

Added tests/_async/test_file_api.py and tests/_sync/test_file_api.py (3 tests each, mock-based — no network):

• upload closes the handle it opened on success (regression)

• update closes the handle it opened on error

• update closes the handle it opened on error

Verified: reverting only the source change fails the tests on close not being called. All 6 pass; ruff check and

Verified: reverting only the source change fails the tests on close not being called. All 6 pass; ruff check and
ruff format clean.

Note: the diff on file_api.py looks larger than it is — wrapping the body in try reindents it. The only
behavioural change is where the close happens.

Note: the diff on file_api.py looks larger than it is — wrapping the body in try reindents it. The only
behavioural change is where the close happens.

[chengwudi1]

Hi! Friendly ping — the CI workflows (CI/CD, title check, SDK compliance) are
waiting on approval to run for this first-time contributor PR. Could you approve
them? Happy to adjust anything. Thanks!

[grdsdev]

Superseded by #1607, which credits you as co-author.

Your diagnosis and your test coverage were both right — update() sharing _upload_or_update, and keeping a success-path regression test, are carried over as-is.

Two things came up while rebasing this onto main, which is why it ended up as a new PR rather than a push to your branch:

1. It no longer applied. #1577 landed in the meantime and rewrote the same block in _request (except (KeyError, TypeError, ValueError), exc.response.text, exc.response.status_code). After resolving that, your _error_response() fixture failed on AttributeError: Mock object has no attribute 'status_code' — httpx sets status_code in __init__, so Mock(spec=Response) rejects it. Nothing you did wrong; it only appeared once #1577 merged.

2. try/finally around _request closes handles it does not own. This is the substantive change in #1607. _request sees only a BufferedReader and cannot tell a handle storage3 opened from a stream the caller passed in, so it closes both. On main that already affects callers who supply their own stream on the success path; wrapping _request extends it to the error path, which breaks retry-after-failure:

with open("big.bin", "rb") as f:
    try:
        client.storage.from_("bucket").upload("big.bin", f)
    except StorageApiError:
        f.seek(0)   # ValueError: I/O operation on closed file
        client.storage.from_("bucket").upload("big.bin", f)

#1607 moves the close into _upload_or_update and upload_to_signed_url instead, which know whether they opened the file.

One more thing your PR surfaced, worth its own issue: run-unasync.py emits from unittest.mock import SyncMock (no such name) for any test using AsyncMock, so the _sync test copies in this package are all hand-corrected and the script breaks them if run today. Your hand-fixed _sync/test_file_api.py was correct precisely because the generator is not. The new test file avoids AsyncMock so it actually round-trips.

Thanks for the work here.