Repository navigation
fix: retry uploads that receive an HTTP 408 (request timeout) - #618
Conversation
sophiecarreras
left a comment
There was a problem hiding this comment.
Thanks, this fixes the right thing: uploads use try_count=1, so the retry happens in UploadManager with a fresh upload URL, which is what we want after a 408. Three non-blocking requests: (1) please export RequestTimeout from b2sdk.v3.exception next to ServiceError so callers can catch it; (2) the name sits very close to the existing B2RequestTimeout/B2RequestTimeoutDuringUpload (client-side timeouts), so a docstring cross-reference or a clearer name would help; (3) consider a test that drives the upload manager through a 408 followed by success and asserts a new upload URL was fetched. CI: all unit sessions are green; the red jobs are the known test_encryption integration failures (#614), unrelated to this change.
Context
An upload that receives an HTTP
408 request_timeoutresponse fails hard: the client raisesUnknownErrorto the caller after exactly one attempt, with no retry and no fresh upload URL.Every other transient upload status —
401,500,503— is retried. Backblaze's IntegrationChecklist lists 408 as a recognized, recoverable upload failure after which the client should
fetch a new upload URL and retry, so customers on flaky networks currently see spurious upload
failures the SDK is meant to absorb.
Root cause
In
b2sdk/_internal/exception.py,interpret_b2_errorhas anelifbranch for each status ittreats specially (the 4xx codes, 429, and the whole 5xx range) but no branch for 408, so a
408 falls through to the unconditional
return UnknownError(...).UnknownErroris a plainB2SimpleError— it does not mix inTransientErrorMixin, so bothshould_retry_upload()andshould_retry_http()inheritFalsefrom the base. The upload manager's retry gate checksshould_retry_upload()and raises immediately, so the upload is never retried.Fix
class RequestTimeout(TransientErrorMixin, B2Error)next toServiceError(the 5xxclass), so both retry predicates are
True.elif status == 408: return RequestTimeout('%d %s %s' % (status, code, message))immediately before the
429branch, grouping 408 with the other transient/retryable statuses.No change to the upload manager or
try_countwiring — the existing retry gate is correct; itsimply never received a retryable exception for 408.
Verification
test/unit/test_exception.py:interpret_b2_error(408, 'request_timeout', 'request timeout', {})returnsRequestTimeoutwithshould_retry_upload()andshould_retry_http()bothTrue. Red before the change, green after.nox -s unit).upload.retry_408recovery scenario goesFAIL → PASS (the client now fetches a new upload URL and retries).
Fixes #605