fix(scope): retry transient remote-fetch failures; fix unhandled rejection in findFirstEnv

[luvkapur]

Why

While load-testing bit ci pr on a full-cascade lane (#10602), a single transient HTTP 500 from one scope fetch (teambit.dependencies) killed a 57-minute build — twice over:

1. objects-fetcher has no retry: one 500 on an idempotent read fails the whole command.
2. The error also escaped as an unhandled promise rejection through findFirstEnv's pLocate — concurrent predicates that reject after pLocate settles have no handler and crash the process.

Changes

• Bounded retry on transient fetch failures: UnexpectedNetworkError (server 5xx / isError payloads) retries up to 3 attempts with 3s/12s backoff, with a warning log per retry. Auth/scope-not-found errors throw immediately as before. Fetch is read-only and idempotent, so retrying is safe.
• findFirstEnv predicate hardened: load failures inside the discovery heuristic log a warning and return false ("cannot be identified as an env") instead of racing an uncatchable rejection.

Verification

• npm run lint green (oxlint + tsc), bit compile on both components.

🤖 Generated with Claude Code

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Retry transient remote scope fetch failures and harden findFirstEnv against late rejections

🐞 Bug fix ✨ Enhancement 🕐 20-40 Minutes

AI Description

• Retry transient 5xx/isError remote scope fetch failures with bounded backoff to avoid CI aborts.
• Prevent unhandled promise rejections in findFirstEnv by catching predicate load failures.
• Add warning logs for retries and skipped env candidates to aid load-test diagnostics.

Diagram

graph TD
  A["bit ci pr"] --> B["ObjectsFetcher"] --> C["fetchFromRemoteWithRetry"] --> D["Remote.fetch"] --> E{{"Remote scope"}}
  A --> F["findFirstEnv (Env runtime)"] --> G["pLocate predicate"] --> H["getEnvComponentByEnvId"] --> D
  C --> I[/"warn: retry backoff"/]
  G --> I

  subgraph Legend
    direction LR
    _mod["Module/flow"] ~~~ _ext{{"External"}} ~~~ _log[/"Log"/]
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Use a shared retry utility (e.g., p-retry) for all network reads

• ➕ Centralizes retry/backoff policy and telemetry
• ➕ Reduces risk of inconsistent retry behavior across modules
• ➖ Introduces/expands dependency footprint or shared utility surface
• ➖ May be overkill for a single hot path needing immediate hardening

2. Make env discovery predicates sequential (avoid concurrent pLocate hazards)

• ➕ Eliminates late-rejection class entirely by design
• ➕ Simplifies error handling and logging
• ➖ Potentially slower env resolution due to loss of concurrency
• ➖ Behavioral change in discovery performance characteristics

3. Add retry at Remote.fetch transport layer instead of ObjectsFetcher

• ➕ Covers all callers uniformly, not just objects-fetcher
• ➕ Keeps higher-level code simpler
• ➖ Harder to scope retry to truly idempotent operations
• ➖ Risk of retrying non-idempotent calls if not carefully segmented

Recommendation: The PR’s approach is a good targeted fix: retry is added only around an idempotent read path (objects fetch) and only for the transient error class (UnexpectedNetworkError), while findFirstEnv is made robust against pLocate’s concurrent predicate late-rejection behavior. Consider later consolidating retry logic into a shared network-read policy if similar incidents occur in other fetch paths.

Files changed (2) +40 / -6

Bug fix (2) +40 / -6

objects-fetcher.tsAdd bounded retry/backoff for transient remote.fetch failures +25/-1

Add bounded retry/backoff for transient remote.fetch failures

• Routes remote scope fetches through a new helper that retries UnexpectedNetworkError up to 3 attempts with exponential backoff (3s, 12s). Logs a warning on each retry while preserving immediate failure for non-transient errors.

components/legacy/scope/objects-fetcher/objects-fetcher.ts

environments.main.runtime.tsCatch pLocate predicate failures in findFirstEnv to avoid unhandled rejections +15/-5

Catch pLocate predicate failures in findFirstEnv to avoid unhandled rejections

• Wraps the async predicate body in a try/catch so late rejections from concurrent pLocate evaluation cannot crash the process. On failure to load an env component, logs a warning and returns false (treating it as “not an env”).

scopes/envs/envs/environments.main.runtime.ts

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📜 Skill insights (0)

1. Retry blocks fetch concurrency 🐞 Bug ➹ Performance ⭐ New

Description

ObjectFetcher.fetchFromRemoteWithRetry awaits backoff delays inside the per-scope task run under
pMapPool concurrency, so any scope that is retrying continues to occupy a concurrency slot while
sleeping. When the pool is saturated, this can delay starting fetches for other scopes and increase
overall import wall-clock time even if most scopes are healthy.

Code

components/legacy/scope/objects-fetcher/objects-fetcher.ts[R202-205]

+            delayMs / 1000
+          }s. error: ${err.message}`
+        );
+        await new Promise((resolve) => setTimeout(resolve, delayMs));

Evidence

The fetch workflow runs each scope’s fetch inside a bounded-concurrency pMapPool task; the new retry
logic introduces an awaited delay (setTimeout) inside that task. While the delay is pending, the
pool slot remains in use, which can delay scheduling of other scopes when the pool is saturated.

components/legacy/scope/objects-fetcher/objects-fetcher.ts[50-80]
components/legacy/scope/objects-fetcher/objects-fetcher.ts[186-208]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`fetchFromRemoteWithRetry()` performs `setTimeout` backoff inside the same async function awaited by the per-scope fetch task. Because `fetchFromRemoteAndWrite()` uses a bounded concurrency pool (`pMapPool(..., { concurrency })`), a task that is waiting to retry can hold a concurrency slot and prevent other scopes from even starting their first attempt.

### Issue Context
- The per-scope fetches are executed under a bounded concurrency pool.
- The retry logic uses `await new Promise(resolve => setTimeout(resolve, delayMs))`, which keeps that pool slot occupied during the sleep.

### Fix Focus Areas
- components/legacy/scope/objects-fetcher/objects-fetcher.ts[50-80]
- components/legacy/scope/objects-fetcher/objects-fetcher.ts[186-208]

### Suggested fix approach
Restructure retries so the backoff waiting does **not** happen while holding a `pMapPool` worker slot. One practical pattern:
1. Run an initial pass over all scopes (attempt #1) under the existing concurrency.
2. Collect scopes that failed with retryable errors.
3. Perform retry rounds for only the failed scopes (with the same concurrency limit), applying the backoff delay **between rounds** (or scheduling retries) so other scopes aren’t prevented from starting their first attempt.

This preserves bounded parallelism for active network calls while avoiding concurrency starvation during backoff.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Overbroad network retry 🐞 Bug ☼ Reliability

Description

ObjectFetcher.fetchFromRemoteWithRetry retries purely on err instanceof UnexpectedNetworkError,
but UnexpectedNetworkError is also used as the default wrapper for remote non-ok responses with
missing/unrecognized error codes, not only transient 5xx conditions. This can add avoidable backoff
delays (and warn logs) for permanent failures that will never succeed on retry.

Code

components/legacy/scope/objects-fetcher/objects-fetcher.ts[R198-201]

+        if (!(err instanceof UnexpectedNetworkError) || attempt >= maxAttempts) throw err;
+        const delayMs = 3000 * 4 ** (attempt - 1); // 3s, then 12s
+        logger.warn(
+          `fetch from "${scopeName}" failed with a network error (attempt ${attempt}/${maxAttempts}), retrying in ${

Evidence

The retry decision is based solely on UnexpectedNetworkError, while the network layer creates
UnexpectedNetworkError as the default mapping for missing/unrecognized error codes, meaning this
retry path can be taken for more than transient 5xx responses.

components/legacy/scope/objects-fetcher/objects-fetcher.ts[192-206]
scopes/scope/network/remote-error-handler.ts[12-16]
scopes/scope/network/http/http.ts[448-465]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`fetchFromRemoteWithRetry()` retries any `UnexpectedNetworkError`, but `UnexpectedNetworkError` is constructed as a broad default for unknown/missing remote error codes. This can cause retries + backoff for non-transient failures.
## Issue Context
- `remoteErrorHandler()` returns `UnexpectedNetworkError` in its default branch (including when `code` is undefined).
- `Http.throwForNonOkStatus()` calls `remoteErrorHandler(error?.code, ...)`, so unmapped responses become `UnexpectedNetworkError` regardless of HTTP status.
## Fix Focus Areas
- Add explicit retryability metadata (e.g. `statusCode`, `isRetryable`, `isServerError`) onto `UnexpectedNetworkError` at creation time (ideally in `throwForNonOkStatus()` / `remoteErrorHandler()` where status/code are known).
- Update `fetchFromRemoteWithRetry()` to retry only when that metadata indicates a transient condition (e.g. HTTP 5xx / server busy), and fail fast otherwise.
### Fix Focus Areas (code pointers)
- components/legacy/scope/objects-fetcher/objects-fetcher.ts[192-206]
- scopes/scope/network/http/http.ts[448-465]
- scopes/scope/network/remote-error-handler.ts[12-16]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Env discovery swallows errors 🐞 Bug ≡ Correctness

Description

findFirstEnv() now catches all failures from getEnvComponentByEnvId() and returns false, which
can mask real env-loading/configuration errors and cause callers to proceed to another/default env
instead of surfacing the underlying problem. Since the candidate list is derived from all
extension/aspect IDs, a systemic load issue can also produce many warnings (one per failing ID) in a
single run.

Code

scopes/envs/envs/environments.main.runtime.ts[R989-992]

+      } catch (err: any) {
+        this.logger.warn(`findFirstEnv: failed loading env-component "${id}", skipping it. error: ${err.message}`);
+        return false;
+      }

Evidence

The new catch converts any getEnvComponentByEnvId failure into a negative predicate result,
while getEnvComponentByEnvId explicitly throws on env load failures; plus the ID list feeding
findFirstEnv is built from all extensions, increasing warning volume when a systemic load issue
occurs.

scopes/envs/envs/environments.main.runtime.ts[965-993]
scopes/envs/envs/environments.main.runtime.ts[503-510]
scopes/envs/envs/environments.main.runtime.ts[940-949]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`findFirstEnv()` now swallows *all* errors from `getEnvComponentByEnvId()` and treats them as "not an env". This prevents unhandled rejections from concurrent predicates, but it also suppresses legitimate configuration/logic failures and can create noisy per-ID warnings.
## Issue Context
- `getEnvComponentByEnvId()` throws a `BitError` when the env component can’t be loaded.
- `findFirstEnv()` is fed by a list built from all extensions/aspects, so repeated load failures can emit many warnings.
## Fix Focus Areas
- Avoid concurrent predicate rejections without blanket error swallowing:
- Option A: replace `pLocate` with a simple sequential `for...of` that awaits each check (prevents unhandled rejections by construction) and can selectively `catch` only expected load/transient errors.
- Option B: if keeping concurrency, ensure every predicate promise is fully handled (e.g., wrap predicate body so it *always* resolves and separately collect/report the first error encountered).
- Narrow the `catch` behavior:
- Continue (return false) only for expected env-load failures (e.g. known network/transient errors / "can't load env"), and for other error types include richer logging (stack) and/or a single aggregated warning to avoid log spam.
### Fix Focus Areas (code pointers)
- scopes/envs/envs/environments.main.runtime.ts[965-993]
- scopes/envs/envs/environments.main.runtime.ts[503-510]
- scopes/envs/envs/environments.main.runtime.ts[940-949]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous review results

Review updated until commit 59d94cb ⚖️ Balanced

Results up to commit e851c0a

🐞 Bugs (2) 📘 Rule violations (0) 📜 Skill insights (0)

1. Overbroad network retry 🐞 Bug ☼ Reliability

Description

ObjectFetcher.fetchFromRemoteWithRetry retries purely on err instanceof UnexpectedNetworkError,
but UnexpectedNetworkError is also used as the default wrapper for remote non-ok responses with
missing/unrecognized error codes, not only transient 5xx conditions. This can add avoidable backoff
delays (and warn logs) for permanent failures that will never succeed on retry.

Code

components/legacy/scope/objects-fetcher/objects-fetcher.ts[R198-201]

+        if (!(err instanceof UnexpectedNetworkError) || attempt >= maxAttempts) throw err;
+        const delayMs = 3000 * 4 ** (attempt - 1); // 3s, then 12s
+        logger.warn(
+          `fetch from "${scopeName}" failed with a network error (attempt ${attempt}/${maxAttempts}), retrying in ${

Evidence

The retry decision is based solely on UnexpectedNetworkError, while the network layer creates
UnexpectedNetworkError as the default mapping for missing/unrecognized error codes, meaning this
retry path can be taken for more than transient 5xx responses.

components/legacy/scope/objects-fetcher/objects-fetcher.ts[192-206]
scopes/scope/network/remote-error-handler.ts[12-16]
scopes/scope/network/http/http.ts[448-465]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`fetchFromRemoteWithRetry()` retries any `UnexpectedNetworkError`, but `UnexpectedNetworkError` is constructed as a broad default for unknown/missing remote error codes. This can cause retries + backoff for non-transient failures.

## Issue Context
- `remoteErrorHandler()` returns `UnexpectedNetworkError` in its default branch (including when `code` is undefined).
- `Http.throwForNonOkStatus()` calls `remoteErrorHandler(error?.code, ...)`, so unmapped responses become `UnexpectedNetworkError` regardless of HTTP status.

## Fix Focus Areas
- Add explicit retryability metadata (e.g. `statusCode`, `isRetryable`, `isServerError`) onto `UnexpectedNetworkError` at creation time (ideally in `throwForNonOkStatus()` / `remoteErrorHandler()` where status/code are known).
- Update `fetchFromRemoteWithRetry()` to retry only when that metadata indicates a transient condition (e.g. HTTP 5xx / server busy), and fail fast otherwise.

### Fix Focus Areas (code pointers)
- components/legacy/scope/objects-fetcher/objects-fetcher.ts[192-206]
- scopes/scope/network/http/http.ts[448-465]
- scopes/scope/network/remote-error-handler.ts[12-16]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Env discovery swallows errors 🐞 Bug ≡ Correctness

Description

findFirstEnv() now catches all failures from getEnvComponentByEnvId() and returns false, which
can mask real env-loading/configuration errors and cause callers to proceed to another/default env
instead of surfacing the underlying problem. Since the candidate list is derived from all
extension/aspect IDs, a systemic load issue can also produce many warnings (one per failing ID) in a
single run.

Code

scopes/envs/envs/environments.main.runtime.ts[R989-992]

+      } catch (err: any) {
+        this.logger.warn(`findFirstEnv: failed loading env-component "${id}", skipping it. error: ${err.message}`);
+        return false;
+      }

Evidence

The new catch converts any getEnvComponentByEnvId failure into a negative predicate result,
while getEnvComponentByEnvId explicitly throws on env load failures; plus the ID list feeding
findFirstEnv is built from all extensions, increasing warning volume when a systemic load issue
occurs.

scopes/envs/envs/environments.main.runtime.ts[965-993]
scopes/envs/envs/environments.main.runtime.ts[503-510]
scopes/envs/envs/environments.main.runtime.ts[940-949]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`findFirstEnv()` now swallows *all* errors from `getEnvComponentByEnvId()` and treats them as "not an env". This prevents unhandled rejections from concurrent predicates, but it also suppresses legitimate configuration/logic failures and can create noisy per-ID warnings.

## Issue Context
- `getEnvComponentByEnvId()` throws a `BitError` when the env component can’t be loaded.
- `findFirstEnv()` is fed by a list built from all extensions/aspects, so repeated load failures can emit many warnings.

## Fix Focus Areas
- Avoid concurrent predicate rejections without blanket error swallowing:
 - Option A: replace `pLocate` with a simple sequential `for...of` that awaits each check (prevents unhandled rejections by construction) and can selectively `catch` only expected load/transient errors.
 - Option B: if keeping concurrency, ensure every predicate promise is fully handled (e.g., wrap predicate body so it *always* resolves and separately collect/report the first error encountered).
- Narrow the `catch` behavior:
 - Continue (return false) only for expected env-load failures (e.g. known network/transient errors / "can't load env"), and for other error types include richer logging (stack) and/or a single aggregated warning to avoid log spam.

### Fix Focus Areas (code pointers)
- scopes/envs/envs/environments.main.runtime.ts[965-993]
- scopes/envs/envs/environments.main.runtime.ts[503-510]
- scopes/envs/envs/environments.main.runtime.ts[940-949]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-free-for-open-source-projects]

Code review by qodo was updated up to the latest commit 59d94cb