fix(storage): close the upload handle we opened, and only that one

[grdsdev]

Supersedes #1586 and the closed #1575.

The leak

_request closed the uploaded file handle after the except HTTPStatusError block, so the close only ran on success. Any non-2xx raised StorageApiError straight past it and the handle stayed open. The handle only ever existed inside the local files mapping, so the caller had no way to close it either — Python reports it as ResourceWarning: unclosed file. Both upload() / update() (via _upload_or_update) and upload_to_signed_url() are affected.

Credit for the diagnosis goes to @tushardev-365 in #1575, including the fd-count reproduction and the note that a retained traceback keeps the frame holding the reader alive.

Why not just try/finally around _request

That is what both #1575 and #1586 did, and it does close the leak. But it leaves the close in a frame that cannot tell the two cases apart:

if files and "file" in files and isinstance(files["file"][1], BufferedReader):
    files["file"][1].close()

_request sees only a BufferedReader. A handle storage3 opened from a path and a stream the caller passed in are both BufferedReader, so it closes both. On main that already bites callers who supply their own stream — it is closed under them on the success path. Wrapping _request in try/finally extends that to the error path too, 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)

This change

Move the close into the two functions that open the file — _upload_or_update and upload_to_signed_url — which know whether they own the handle:

if isinstance(file, BufferedReader) or isinstance(file, bytes) or isinstance(file, FileIO):
    # bytes or byte-stream-like object received -- the caller owns it
    opened = None
    files = {"file": (filename, file, content_type)}
else:
    # str or pathlib.path received -- we own the handle, so we close it
    opened = open(file, "rb")
    files = {"file": (filename, opened, content_type)}

try:
    response = await self._request(...)
finally:
    if opened is not None:
        opened.close()

_request no longer touches the handle at all.

Result: a handle storage3 opened is closed on both paths; a caller-supplied stream is left alone on both paths.

Tests

tests/_async/test_file_api.py and its generated _sync twin, 4 tests each:

• test_upload_closes_file_handle_on_error
• test_upload_closes_file_handle_on_success (regression)
• test_update_closes_file_handle_on_error — update() shares _upload_or_update
• test_caller_supplied_handle_is_left_open — asserts on both paths

They assert on the real handle's .closed (captured from the files mapping the client received) rather than patching builtins.open and asserting on a mock. That approach is from #1575.

Reverting only the source change turns 6 of the 8 red, including the caller-supplied guard on the success path, which fails on main today.

Note for reviewers

run-unasync.py cannot regenerate the existing storage test files: it emits from unittest.mock import SyncMock (no such name) wherever a test uses AsyncMock, so tests/_sync/test_bucket.py and tests/_sync/test_client.py are hand-corrected and running the script today breaks them. The new test file avoids AsyncMock entirely, so its _sync copy is genuinely generated and round-trips. Worth a separate issue.

Verification

• 74 storage unit tests pass
• ruff check and ruff format --check clean
• mypy shows only the pre-existing pyiceberg import-not-found errors in analytics.py, untouched here

[github-actions]

⚠️ Capability matrix drift detected

The following capabilities are marked implemented in the matrix but could not be found in python:

• database.configuration.auto_retry → expected symbol: AsyncSelectRequestBuilder.retry
• database.configuration.auto_retry → expected symbol: SyncSelectRequestBuilder.retry
• realtime.client.connect → expected symbol: SyncRealtimeClient.connect
• realtime.client.disconnect → expected symbol: AsyncRealtimeClient.disconnect
• realtime.client.disconnect → expected symbol: SyncRealtimeClient.disconnect
• realtime.channel.subscribe → expected symbol: SyncRealtimeChannel.subscribe
• realtime.channel.unsubscribe → expected symbol: SyncRealtimeChannel.unsubscribe
• realtime.channel.broadcast → expected symbol: SyncRealtimeChannel.send_broadcast
• realtime.subscriptions.broadcast → expected symbol: SyncRealtimeChannel.on_broadcast
• realtime.subscriptions.postgres_changes → expected symbol: SyncRealtimeChannel.on_postgres_changes
• realtime.subscriptions.subscribe_presence → expected symbol: AsyncRealtimeChannel.on_presence_change
• realtime.subscriptions.subscribe_presence → expected symbol: SyncRealtimeChannel.on_presence_change
• realtime.presence.track → expected symbol: SyncRealtimeChannel.track
• realtime.presence.untrack → expected symbol: SyncRealtimeChannel.untrack
• realtime.presence.presence_state → expected symbol: AsyncRealtimeChannel.presences
• realtime.presence.presence_state → expected symbol: SyncRealtimeChannel.presences

The following capabilities are marked implemented in python but have no registered symbols to verify:

• auth.session.auto_refresh (no symbols list — cannot confirm implementation exists)
• client.authentication_integration.cross_client_token_sync (no symbols list — cannot confirm implementation exists)
• client.authentication_integration.oauth_flow_type (no symbols list — cannot confirm implementation exists)
• client.session_management.custom_storage (no symbols list — cannot confirm implementation exists)
• client.session_management.persist_session (no symbols list — cannot confirm implementation exists)
• client.request_configuration.custom_http_client (no symbols list — cannot confirm implementation exists)
• client.request_configuration.global_headers (no symbols list — cannot confirm implementation exists)
• database.mutate.select_after_mutation (no symbols list — cannot confirm implementation exists)
• database.using_filters.eq (no symbols list — cannot confirm implementation exists)
• database.using_filters.neq (no symbols list — cannot confirm implementation exists)
• database.using_filters.gt (no symbols list — cannot confirm implementation exists)
• database.using_filters.gte (no symbols list — cannot confirm implementation exists)
• database.using_filters.lt (no symbols list — cannot confirm implementation exists)
• database.using_filters.lte (no symbols list — cannot confirm implementation exists)
• database.using_filters.like (no symbols list — cannot confirm implementation exists)
• database.using_filters.ilike (no symbols list — cannot confirm implementation exists)
• database.using_filters.is (no symbols list — cannot confirm implementation exists)
• database.using_filters.in (no symbols list — cannot confirm implementation exists)
• database.using_filters.contains (no symbols list — cannot confirm implementation exists)
• database.using_filters.contained_by (no symbols list — cannot confirm implementation exists)
• database.using_filters.range_gt (no symbols list — cannot confirm implementation exists)
• database.using_filters.range_gte (no symbols list — cannot confirm implementation exists)
• database.using_filters.range_lt (no symbols list — cannot confirm implementation exists)
• database.using_filters.range_lte (no symbols list — cannot confirm implementation exists)
• database.using_filters.range_adjacent (no symbols list — cannot confirm implementation exists)
• database.using_filters.overlaps (no symbols list — cannot confirm implementation exists)
• database.using_filters.text_search (no symbols list — cannot confirm implementation exists)
• database.using_filters.match (no symbols list — cannot confirm implementation exists)
• database.using_filters.not (no symbols list — cannot confirm implementation exists)
• database.using_filters.or (no symbols list — cannot confirm implementation exists)
• database.using_filters.raw (no symbols list — cannot confirm implementation exists)
• database.using_filters.regex (no symbols list — cannot confirm implementation exists)
• database.using_filters.regex_icase (no symbols list — cannot confirm implementation exists)
• database.using_filters.not_in (no symbols list — cannot confirm implementation exists)
• database.using_modifiers.order (no symbols list — cannot confirm implementation exists)
• database.using_modifiers.limit (no symbols list — cannot confirm implementation exists)
• database.using_modifiers.range (no symbols list — cannot confirm implementation exists)
• database.using_modifiers.request_cancellation (no symbols list — cannot confirm implementation exists)
• database.using_modifiers.relationship_embed (no symbols list — cannot confirm implementation exists)
• database.configuration.request_timeout (no symbols list — cannot confirm implementation exists)
• functions.invocation.region_selection (no symbols list — cannot confirm implementation exists)
• functions.invocation.request_cancellation (no symbols list — cannot confirm implementation exists)
• functions.invocation.timeout (no symbols list — cannot confirm implementation exists)
• realtime.subscriptions.postgres_changes_filter (no symbols list — cannot confirm implementation exists)
• realtime.subscriptions.private_channel (no symbols list — cannot confirm implementation exists)
• realtime.subscriptions.broadcast_self (no symbols list — cannot confirm implementation exists)
• realtime.subscriptions.broadcast_replay (no symbols list — cannot confirm implementation exists)
• realtime.presence.presence_key (no symbols list — cannot confirm implementation exists)
• realtime.configuration.heartbeat_interval (no symbols list — cannot confirm implementation exists)
• storage.file_buckets.access_bucket (no symbols list — cannot confirm implementation exists)

These may have been renamed, removed, or never registered. Please update the capability matrix.
See: https://github.com/supabase/sdk/blob/main/packages/capability-matrix/docs/capability-matrix.md