added retry logic for object storage download failures

[AarishMansur]

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.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 455398a

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

[coderabbitai]

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 da00c3a and ced1c90.

📒 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

apps/webapp/app/v3/r2.server.ts (1)

128-128: Consider making retry parameters configurable.

The call uses hardcoded default values (3 retries, 500ms delay). For production environments, you may want to make these configurable via environment variables to tune retry behavior without code changes.

Example: Add configuration to env.server.ts:

// In env.server.ts schema
OBJECT_STORE_RETRY_COUNT: z.coerce.number().default(3),
OBJECT_STORE_RETRY_DELAY_MS: z.coerce.number().default(500),

Then use them:

-    const response = await fetchWithRetry(url.toString());
+    const response = await fetchWithRetry(
+      url.toString(), 
+      env.OBJECT_STORE_RETRY_COUNT, 
+      env.OBJECT_STORE_RETRY_DELAY_MS
+    );

As per coding guidelines (access environment variables only via the env export from app/env.server.ts).

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9fdf91a and 7b08bba.

📒 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

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7b08bba and da00c3a.

📒 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)

140-140: Integration looks correct.

The integration of fetchWithRetry to replace the direct r2.fetch call is syntactically correct. Once the critical retry logic issue in fetchWithRetry is fixed, this change will properly add resilience to transient object storage failures.