trigger.dev icon indicating copy to clipboard operation
trigger.dev copied to clipboard

added retry logic for object storage download failures

Open AarishMansur opened this issue 2 months ago • 2 comments

Closes #2484

This PR introduces a retry mechanism in the downloadPacketFromObjectStore function to handle transient network or object storage errors.

Changes Made :

  • Added a new helper function fetchWithRetry() inside objectStore.ts.

  • Wrapped r2.fetch() calls with retry logic to automatically reattempt failed requests.

  • Logged each retry attempt using the existing logger for better visibility.

  • Added proper error handling for non-retriable errors (4xx responses).

Testing :

I have not fully tested this locally due to Codespaces limitations and missing object storage setup. Logic compiles successfully and builds with no TypeScript errors.

Maintainers can confirm runtime behavior during review.

AarishMansur avatar Nov 09 '25 13:11 AarishMansur

⚠️ No Changeset found

Latest commit: 73d29b6b2be7bd80c17c1a8e818ea933360dec7e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

changeset-bot[bot] avatar Nov 09 '25 13:11 changeset-bot[bot]

Walkthrough

The change modifies downloadPacketFromObjectStore in apps/webapp/app/v3/r2.server.ts to replace a single r2.fetch call with a new fetchWithRetry(url, retries = 3, delay = 500) helper. fetchWithRetry classifies 4xx responses as non-retryable, treats 5xx responses and network errors as retryable, performs per-attempt backoff with logged warnings on retries, and throws after exhausting retries. On success it returns the successful response; subsequent reading of the response body is unchanged.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Review fetchWithRetry for correct retry loop, attempt counting, and backoff calculation
  • Verify HTTP status handling (4xx vs 5xx) matches intended retry policy
  • Check logging messages include sufficient context (URL, attempt, status/error)
  • Confirm thrown errors on exhaustion surface useful diagnostics
  • Confirm response body reading after a successful retry is unchanged and safe
  • File to inspect: apps/webapp/app/v3/r2.server.ts

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding retry logic for object storage download failures, matching the core objective of the PR.
Description check ✅ Passed The description covers the key aspects: issue reference, changes made, and testing notes. However, it lacks the full structure from the template including checklist verification and changelog sections.
Linked Issues check ✅ Passed The PR implements the retry mechanism for object storage downloads as specified in issue #2484, with fetchWithRetry logic handling transient errors and proper error classification.
Out of Scope Changes check ✅ Passed All changes are scoped to the downloadPacketFromObjectStore function and the new fetchWithRetry helper, directly addressing the transient error retry objective from issue #2484.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between da00c3ae3361c021e35679444e0cc70d0f1aff68 and ced1c90f1cca11d388be666989545807fe4e6900.

📒 Files selected for processing (1)
  • apps/webapp/app/v3/r2.server.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code For TypeScript, we usually use types over interfaces Avoid enums No default exports, use function declarations

Files:

  • apps/webapp/app/v3/r2.server.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

We use zod a lot in packages/core and in the webapp

Files:

  • apps/webapp/app/v3/r2.server.ts
apps/webapp/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)

When importing from @trigger.dev/core in the webapp, never import the root package path; always use one of the documented subpath exports from @trigger.dev/core’s package.json

Files:

  • apps/webapp/app/v3/r2.server.ts
{apps/webapp/app/**/*.server.{ts,tsx},apps/webapp/app/routes/**/*.ts}

📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)

Access environment variables only via the env export from app/env.server.ts; do not reference process.env directly

Files:

  • apps/webapp/app/v3/r2.server.ts
apps/webapp/app/**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)

Modules intended for test consumption under apps/webapp/app/**/*.ts must not read environment variables; accept configuration via options instead

Files:

  • apps/webapp/app/v3/r2.server.ts
🔇 Additional comments (1)
apps/webapp/app/v3/r2.server.ts (1)

104-149: Nice retry guard.
4xx paths now short-circuit via NonRetryableError, while server/network failures retry with bounded backoff and clear logging, which matches the transient-error objective perfectly.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Nov 09 '25 13:11 coderabbitai[bot]

@AarishMansur thanks for the PR but we generally don't accept untested PRs as it's too much work for maintainers.

ericallam avatar Nov 24 '25 14:11 ericallam