Fix multipart upload part retries missing connection exceptions

[BryanFauble]

Problem:

A user reported an upload failing with SynapseUploadFailedException: Part upload failed only 3 seconds into a 10.5GB transfer — far too fast to have exhausted the intended part-level retry window.

Tracing the retry logic in multipart_upload_async.py found the root cause: _put_part_with_retry (async path) passes retry_exceptions=[requests.exceptions.ConnectionError] to with_retry_time_based, but the session performing the PUT is an httpx.Client, not a requests.Session. httpx never raises requests.exceptions.ConnectionError — it raises httpx.ConnectError, httpx.ReadTimeout, httpx.RemoteProtocolError, etc. The retry matcher (_is_retryable) checks the caught exception's class/class-name against the supplied allowlist, so none of httpx's exception types ever match, and the retry helper's default-filling logic only substitutes a default when the caller's list is empty — it doesn't merge in the correct list when a (wrong) list is already supplied.

Net effect: any genuine transient network error during a part PUT (a real socket/connect/read/timeout failure, not an HTTP status code) skipped the entire time-boxed backoff window and failed the part immediately. The only resilience left was the coarser whole-upload-attempt retry (MAX_RETRIES = 7 in _multipart_upload_async), which restarts the entire UploadAttemptAsync rather than just retrying the one failed PUT.

While auditing every retry_exceptions= call site for the same class of bug, the sync upload path (multipart_upload.py) turned out to have a milder version of the same gap: it correctly uses requests.exceptions.ConnectionError (since it genuinely uses requests.Session), but only that one exception type — missing requests.exceptions.Timeout and ChunkedEncodingError, both of which a slow/stalled part PUT can legitimately raise. The download side (download_async.py, download_threads.py) and client.py's standard retry params already use the full, correct list.

Solution:

• synapseclient/core/upload/multipart_upload_async.py: swapped retry_exceptions=[requests.exceptions.ConnectionError] for the existing RETRYABLE_CONNECTION_EXCEPTIONS list (already defined in core/retry.py and used correctly on the download side). Removed the now-unused import requests.
• synapseclient/core/upload/multipart_upload.py: widened the sync path's retry_exceptions=[requests.exceptions.ConnectionError] to the same RETRYABLE_CONNECTION_EXCEPTIONS list, restoring parity with the async path and the rest of the codebase.

Testing:

tests/unit/synapseclient/core/upload/test_multipart_upload_async.py (new file) covers each distinct path through UploadAttemptAsync._handle_part / _put_part_with_retry:

• Regression test: httpx.ConnectError on the first PUT, success on the second — confirmed this test fails without the fix (the exception propagates immediately with zero retries) and passes with it.
• Retryable HTTP status code (503) → retry → success.
• 403 expired presigned URL → refresh URL → success (confirms the existing URL-refresh path is unaffected).
• Non-retryable exception (ValueError) → fails immediately with no retry (confirms the fix doesn't over-widen retry behavior).

tests/unit/synapseclient/core/upload/unit_test_multipart_upload.py: added test_handle_part__timeout_error, a regression test for the sync path — requests.exceptions.Timeout on the first PUT, success on the second. Confirmed this fails without the sync-path fix and passes with it.

Full upload unit test suite (tests/unit/synapseclient/core/upload/, 29 tests) passes, and pre-commit run --all-files (ruff, isort, black, bandit) passes clean.

[Copilot]

Pull request overview

This pull request fixes part-level retry behavior for async multipart uploads by aligning the retryable exception allowlist with the actual HTTP client in use (httpx.Client). This improves resilience for large uploads by ensuring transient connection-level failures during individual part PUTs correctly use the existing time-boxed retry/backoff logic, rather than failing a part immediately and forcing a full upload-attempt restart.

Changes:

• Use RETRYABLE_CONNECTION_EXCEPTIONS (httpx-aware) for _put_part_with_retry instead of requests.exceptions.ConnectionError.
• Remove the now-unused requests import from the async multipart upload implementation.
• Add unit tests covering connection-error retry, retryable HTTP status retry, presigned URL refresh on 403, and immediate failure for non-retryable exceptions.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
synapseclient/core/upload/multipart_upload_async.py	Fixes part PUT retry behavior by using the shared httpx-compatible retry exception list.
tests/unit/synapseclient/core/upload/test_multipart_upload_async.py	Adds regression and path-coverage tests for _handle_part / _put_part_with_retry retry logic.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[linglp]

LGTM!