Skip to content

fix(media): retry failed uploads#1540

Merged
hassiebp merged 1 commit intomainfrom
fix-retry-media-upload
Feb 25, 2026
Merged

fix(media): retry failed uploads#1540
hassiebp merged 1 commit intomainfrom
fix-retry-media-upload

Conversation

@hassiebp
Copy link
Contributor

@hassiebp hassiebp commented Feb 25, 2026

Important

Add retry mechanism for failed media uploads in MediaManager with tests for retryable and non-retryable HTTP statuses.

  • Behavior:
    • Add retry mechanism for failed media uploads in MediaManager.
    • Introduce _upload_with_status_check function in media_manager.py to handle upload and status check.
    • Update _process_upload_media_job to use _upload_with_status_check with backoff strategy.
    • Log failed upload attempts and retry if status is retryable.
  • Tests:
    • Add test_media_upload_retries_on_retryable_http_status in test_media_manager.py to verify retry on 503 status.
    • Add test_media_upload_gives_up_on_non_retryable_http_status in test_media_manager.py to verify no retry on 403 status.

This description was created by Ellipsis for bc20c1a. You can customize this summary. It will automatically update as commits are pushed.

Disclaimer: Experimental PR review

Greptile Summary

Added retry mechanism for failed media uploads with proper error tracking. The implementation wraps the upload operation in _upload_with_status_check which calls raise_for_status() to ensure HTTP errors are properly caught. Failed uploads are now logged via the media.patch API before the exception is re-raised, ensuring visibility into upload failures even when retries are exhausted.

  • Introduced _upload_with_status_check function that performs the upload and validates HTTP status
  • Updated error handling to log failed uploads via media.patch API with status code, error message, and timing
  • Leverages existing _request_with_backoff retry logic which retries 5xx errors and 429, but gives up on other 4xx errors
  • Added comprehensive tests verifying retry behavior for retryable (503) and non-retryable (403) status codes

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The changes are well-structured and properly tested. The retry logic leverages the existing _request_with_backoff mechanism which already has proper retry/giveup logic for different HTTP status codes. The addition of error logging via media.patch ensures visibility into upload failures. The comprehensive tests verify both retryable and non-retryable scenarios, confirming the implementation works as expected.
  • No files require special attention

Important Files Changed

Filename Overview
langfuse/_task_manager/media_manager.py Added retry logic for failed uploads with proper error logging via media.patch API call
tests/test_media_manager.py New test file with comprehensive coverage for retryable (503) and non-retryable (403) HTTP status scenarios

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Start Upload] --> B[Call _upload_with_status_check]
    B --> C[HTTP PUT to upload_url]
    C --> D[Call raise_for_status]
    D --> E{Status OK?}
    E -->|2xx| F[Return Response]
    E -->|Error| G[Raise HTTPStatusError]
    G --> H{Retryable?}
    H -->|5xx or 429| I[Backoff Retry]
    I --> B
    H -->|4xx except 429| J[Catch in Exception Handler]
    F --> K[Log Success via media.patch]
    J --> L[Log Failure via media.patch]
    L --> M[Re-raise Exception]
    K --> N[Complete]
    M --> O[Error Logged & Propagated]
Loading

Last reviewed commit: bc20c1a

@hassiebp hassiebp enabled auto-merge (squash) February 25, 2026 13:40
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@hassiebp hassiebp merged commit 7ff9522 into main Feb 25, 2026
13 checks passed
@hassiebp hassiebp deleted the fix-retry-media-upload branch February 25, 2026 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant