Network-level fetch failures produce opaque "fetch failed" errors and skip bounded retries #267

Closed
opened 2026-08-18 06:31:01 +00:00 by david · 1 comment
Owner

Summary

When a REST call to an issue-source or MR platform (Jira, GitLab, GitHub, Forgejo) fails at the network level — DNS resolution failure, connection refused, TLS error, timeout, etc. — pi-loop surfaces only Node's generic TypeError: fetch failed message, with no indication of what failed to connect or why. This makes misconfiguration (e.g. a wrong/placeholder base URL) very hard to diagnose from the logs alone. It also means these failures are not retried, contradicting AGENTS.md's documented error-handling policy that transient/network errors on the issue source or model get bounded retries.

Real-world trigger (observed)

A target repo's piloop-config.yaml had a leftover placeholder from piloop-config.example.yaml:

gitlab:
  base-url: https://gitlab.example.com   # never edited; real origin is gitlab.com

Because YAML config takes precedence over inferring the base URL from the git origin host, every GitLab REST call in the mr stage targeted the non-resolvable gitlab.example.com. The resulting log gave no clue what was wrong:

[16:08 mr] INFO creating MR
[16:08 mr] ERROR stage failed (agent-failure): fetch failed
[16:08 setup] INFO pi-loop finished at 2026-08-18T06:08:13.222Z
pi-loop failed at mr (agent-failure): fetch failed

Confirmed via direct reproduction:

await fetch('https://gitlab.example.com/api/v4/x');
// throws TypeError: fetch failed
//   .cause = Error: getaddrinfo ENOTFOUND gitlab.example.com { code: 'ENOTFOUND', hostname: 'gitlab.example.com' }

Node's fetch discards the actionable detail into error.cause, and pi-loop never reads it.

Root cause (traced)

  1. GitLabMrClient.createMr() (and every other REST client built on the shared makeAuthenticatedRequest helper — GitLab/GitHub/Forgejo MR clients, GitLab/GitHub/Forgejo issue clients, the Forgejo merge poller, plus Jira's own structurally-identical request() method in JiraRestClient) calls httpFetch(url, init) inside withRetry().
  2. When httpFetch (raw global fetch) fails at the network level, it throws before an HttpResponse is produced — so the if (!response.ok) branch that maps HTTP status codes to a classified error (MrClientError / IssueSourceError / JiraError with proper kind/retriable) never runs.
  3. The raw TypeError('fetch failed') propagates unchanged. withRetry's isRetriable predicate is always scoped to error instanceof <DomainError> && error.retriable, so this un-classified raw TypeError fails that check and is not retried — it fails after a single attempt.
  4. It bubbles up to classifyError() (src/orchestrator/helpers/classifyError.ts), which has no explicit case for MrClientError/IssueSourceError network failures either — see related gap below — so it falls through to the generic per-stage default (agent-failure for mr/implement/review/remediate/docs, unknown otherwise), and error.json's message is literally whatever error.message was: "fetch failed".

classifyError() currently has no case for MrClientError at all (only for MrStageError, JiraError/IssueSourceError, GitError, config errors). This means even ordinary HTTP-status MR-client errors (401/403/404/429/5xx from mrClientErrorFromStatus) are flattened to generic agent-failure in error.json today, losing their real kind/retriable classification. This should be fixed alongside the network-error work since the fix touches the same code path.

Affected call sites

All REST clients built on src/shared/http/authenticatedFetch.ts's makeAuthenticatedRequest():

  • src/gitlab/services/gitLabMrClient.ts
  • src/gitlab/services/gitLabIssueClient.ts
  • src/github/services/gitHubMrClient.ts
  • src/github/services/gitHubIssueClient.ts
  • src/forgejo/services/forgejoMrClient.ts
  • src/forgejo/services/forgejoIssueClient.ts
  • src/mr/services/forgejoMergePoller.ts

Plus the structurally-identical (but separate) private request() method in:

  • src/jira/services/jiraRestClient.ts

Note: src/mr/services/gitlabMergePoller.ts and src/mr/services/githubMergePoller.ts use CLI executors (glab/gh), not fetch, so they are out of scope for the fetch-specific part of this fix, but see the "Also consider" note below.

Proposed fix

1. Classify network-level fetch failures at the source

Add a shared helper (e.g. src/shared/http/classifyFetchError.ts) that recognizes:

  • Node's TypeError with message fetch failed and inspects error.cause for a code (ENOTFOUND, ECONNREFUSED, ECONNRESET, ETIMEDOUT, EAI_AGAIN, cert/TLS error codes, etc.) and/or hostname
  • DOMException/TimeoutError from AbortSignal.timeout(...) if/when request timeouts are added
  • Falls back to a generic "network error" classification when the cause shape isn't recognized, so unknown network failures are still retried rather than silently swallowed as fail-fast

Produce a clear, actionable message, e.g.:

GitLab MR create for teg/ovation-technology/development/ignition-2.0-dotnet-poc failed: could not reach gitlab.example.com (ENOTFOUND) — check GITLAB_BASE_URL and network connectivity

2. Add a network error kind end-to-end

  • src/shared/http/classifyHttpStatus.ts — add 'network' to HttpErrorKind, retriable true
  • MrClientError (src/mr/helpers/mrClientError.ts), IssueSourceError (src/issue/helpers/issueSourceError.ts), JiraError (src/jira/helpers/jiraError.ts + src/jira/types/jiraError.ts) — all reuse HttpErrorKind/mirror it, so network flows through automatically once added there; verify JiraErrorKind (currently its own separate union) is updated too
  • src/orchestrator/types/orchestrator.ts — add 'network' to ErrorKind (error.json schema)
  • src/artifacts/helpers/validateErrorReport.ts — add 'network' to VALID_KINDS
  • DESIGN.md §3.4 — update the error.json kind enum documentation (currently config|auth|not-found|rate-limit|transient|agent-failure|git|unknown)

3. Wire classification into each request path

In makeAuthenticatedRequest() (src/shared/http/authenticatedFetch.ts), wrap the httpFetch(url, init) call itself (not just the !response.ok branch) in a try/catch that runs the new classifyFetchError helper and throws the caller's errorFactory-equivalent for network errors — likely requires extending AuthenticatedRequestOptions with a networkErrorFactory (or reusing errorFactory with a sentinel status, e.g. 0, mapped to network in classifyHttpStatus). Apply the same treatment to JiraRestClient's separate request() method for consistency.

4. Fix isRetriable predicates to retry network errors

Since network will be retriable: true in the shared classifyHttpStatus/kind policy, the existing isRetriable: (error) => error instanceof MrClientError && error.retriable predicates (and the IssueSourceError/JiraError equivalents) will automatically pick this up once network failures are classified into those same error types — no separate predicate change needed if step 3 is done correctly. Confirm this with a test that a network failure actually retries (per AGENTS.md's bounded-retry policy: 3 attempts, exponential backoff).

5. Fix the classifyError() gap for MrClientError/IssueSourceError

Add explicit cases in src/orchestrator/helpers/classifyError.ts for MrClientError (mirroring the existing JiraError/IssueSourceError case: use error.kind/error.retriable directly) so MR-client failures of any kind (not just network) surface their real classification in error.json instead of the generic per-stage default.

Test plan

  • Unit — classifyFetchError: recognizes ENOTFOUND, ECONNREFUSED, ETIMEDOUT, ECONNRESET, unknown cause.code (generic network fallback), non-TypeError/non-fetch errors (pass through unclassified).
  • Unit — classifyHttpStatus: network kind added, retriable true; existing status-code cases unaffected.
  • Unit — makeAuthenticatedRequest(): a mock httpFetch that throws a TypeError('fetch failed') (with a cause) is caught, classified, and retried up to maxAttempts per the injected retry options; asserts the final thrown error carries kind: 'network', retriable: true, and an actionable message including the host and cause code.
  • Unit — JiraRestClient's request(): same network-failure classification + retry behavior as makeAuthenticatedRequest.
  • Unit — classifyError(): new case for MrClientError mirroring the JiraError/IssueSourceError test — asserts kind/retriable pass through unchanged (covers both network and pre-existing HTTP-status kinds, closing the related gap in item 5).
  • Unit — validateErrorReport: 'network' accepted as a valid kind.
  • Regression — GitLab MR client with a real unreachable host: integration-style test using the existing fake httpFetch seam, simulating a thrown TypeError('fetch failed') from GitLabMrClient.createMr(), asserting the surfaced error message names the unreachable host.

Acceptance criteria

  • A REST call to a DNS-unresolvable or unreachable host produces an error.json with kind: "network", retriable: true, and a message that names the host and the underlying cause (e.g. ENOTFOUND), not the bare string "fetch failed".
  • Such failures are retried per the standard bounded-retry policy (3 attempts, exponential backoff) before failing the stage.
  • MrClientError failures of any kind (not just network) are classified correctly by classifyError() instead of falling back to the generic per-stage agent-failure/unknown default.
  • All existing HTTP-status-based classification (401/403/404/429/5xx) is unaffected — no regression in fail-fast vs retriable behavior for those cases.
  • DESIGN.md §3.4 and error.json schema/tests are updated to include network as a valid kind.

Also consider (out of scope for this issue, noted for future work)

  • src/mr/services/gitlabMergePoller.ts / githubMergePoller.ts shell out to glab/gh CLIs via execFile, not fetch — a similarly opaque CLI-invocation failure (e.g. glab binary missing, or glab itself hitting a DNS failure) would need separate handling in cliMergePollerBase.ts's makeCliRunner catch block, which currently just wraps error.message as kind: 'unknown', retriable: false regardless of cause.
  • No request-level timeout is currently configured on any fetch call (relies on OS-level TCP timeouts, which can be very slow for unreachable-but-routable hosts, e.g. black-holed IPs) — worth a separate issue to add AbortSignal.timeout(...) with a sensible default.

Discovered while investigating

Forgejo issue #265 (fixed): a different, unrelated resume-detection bug where the docs stage was needlessly re-run after a no-op completion, which was masking/compounding the diagnosis of this network-error issue in the same run.

## Summary When a REST call to an issue-source or MR platform (Jira, GitLab, GitHub, Forgejo) fails at the network level — DNS resolution failure, connection refused, TLS error, timeout, etc. — pi-loop surfaces only Node's generic `TypeError: fetch failed` message, with no indication of *what* failed to connect or *why*. This makes misconfiguration (e.g. a wrong/placeholder base URL) very hard to diagnose from the logs alone. It also means these failures are **not retried**, contradicting AGENTS.md's documented error-handling policy that transient/network errors on the issue source or model get bounded retries. ## Real-world trigger (observed) A target repo's `piloop-config.yaml` had a leftover placeholder from `piloop-config.example.yaml`: ```yaml gitlab: base-url: https://gitlab.example.com # never edited; real origin is gitlab.com ``` Because YAML config takes precedence over inferring the base URL from the git origin host, every GitLab REST call in the `mr` stage targeted the non-resolvable `gitlab.example.com`. The resulting log gave no clue what was wrong: ``` [16:08 mr] INFO creating MR [16:08 mr] ERROR stage failed (agent-failure): fetch failed [16:08 setup] INFO pi-loop finished at 2026-08-18T06:08:13.222Z pi-loop failed at mr (agent-failure): fetch failed ``` Confirmed via direct reproduction: ```js await fetch('https://gitlab.example.com/api/v4/x'); // throws TypeError: fetch failed // .cause = Error: getaddrinfo ENOTFOUND gitlab.example.com { code: 'ENOTFOUND', hostname: 'gitlab.example.com' } ``` Node's `fetch` discards the actionable detail into `error.cause`, and pi-loop never reads it. ## Root cause (traced) 1. `GitLabMrClient.createMr()` (and every other REST client built on the shared `makeAuthenticatedRequest` helper — GitLab/GitHub/Forgejo MR clients, GitLab/GitHub/Forgejo issue clients, the Forgejo merge poller, plus Jira's own structurally-identical `request()` method in `JiraRestClient`) calls `httpFetch(url, init)` inside `withRetry()`. 2. When `httpFetch` (raw global `fetch`) fails at the network level, it throws *before* an `HttpResponse` is produced — so the `if (!response.ok)` branch that maps HTTP status codes to a classified error (`MrClientError` / `IssueSourceError` / `JiraError` with proper `kind`/`retriable`) never runs. 3. The raw `TypeError('fetch failed')` propagates unchanged. `withRetry`'s `isRetriable` predicate is always scoped to `error instanceof <DomainError> && error.retriable`, so this un-classified raw `TypeError` fails that check and is **not retried** — it fails after a single attempt. 4. It bubbles up to `classifyError()` (`src/orchestrator/helpers/classifyError.ts`), which has no explicit case for `MrClientError`/`IssueSourceError` network failures either — see related gap below — so it falls through to the generic per-stage default (`agent-failure` for `mr`/`implement`/`review`/`remediate`/`docs`, `unknown` otherwise), and `error.json`'s `message` is literally whatever `error.message` was: `"fetch failed"`. ### Related gap found during investigation `classifyError()` currently has **no case for `MrClientError` at all** (only for `MrStageError`, `JiraError`/`IssueSourceError`, `GitError`, config errors). This means even *ordinary* HTTP-status MR-client errors (401/403/404/429/5xx from `mrClientErrorFromStatus`) are flattened to generic `agent-failure` in `error.json` today, losing their real `kind`/`retriable` classification. This should be fixed alongside the network-error work since the fix touches the same code path. ## Affected call sites All REST clients built on `src/shared/http/authenticatedFetch.ts`'s `makeAuthenticatedRequest()`: - `src/gitlab/services/gitLabMrClient.ts` - `src/gitlab/services/gitLabIssueClient.ts` - `src/github/services/gitHubMrClient.ts` - `src/github/services/gitHubIssueClient.ts` - `src/forgejo/services/forgejoMrClient.ts` - `src/forgejo/services/forgejoIssueClient.ts` - `src/mr/services/forgejoMergePoller.ts` Plus the structurally-identical (but separate) `private request()` method in: - `src/jira/services/jiraRestClient.ts` Note: `src/mr/services/gitlabMergePoller.ts` and `src/mr/services/githubMergePoller.ts` use CLI executors (`glab`/`gh`), not `fetch`, so they are out of scope for the `fetch`-specific part of this fix, but see the "Also consider" note below. ## Proposed fix ### 1. Classify network-level fetch failures at the source Add a shared helper (e.g. `src/shared/http/classifyFetchError.ts`) that recognizes: - Node's `TypeError` with message `fetch failed` and inspects `error.cause` for a `code` (`ENOTFOUND`, `ECONNREFUSED`, `ECONNRESET`, `ETIMEDOUT`, `EAI_AGAIN`, cert/TLS error codes, etc.) and/or `hostname` - `DOMException`/`TimeoutError` from `AbortSignal.timeout(...)` if/when request timeouts are added - Falls back to a generic "network error" classification when the cause shape isn't recognized, so unknown network failures are still retried rather than silently swallowed as fail-fast Produce a clear, actionable message, e.g.: ``` GitLab MR create for teg/ovation-technology/development/ignition-2.0-dotnet-poc failed: could not reach gitlab.example.com (ENOTFOUND) — check GITLAB_BASE_URL and network connectivity ``` ### 2. Add a `network` error kind end-to-end - `src/shared/http/classifyHttpStatus.ts` — add `'network'` to `HttpErrorKind`, retriable `true` - `MrClientError` (`src/mr/helpers/mrClientError.ts`), `IssueSourceError` (`src/issue/helpers/issueSourceError.ts`), `JiraError` (`src/jira/helpers/jiraError.ts` + `src/jira/types/jiraError.ts`) — all reuse `HttpErrorKind`/mirror it, so `network` flows through automatically once added there; verify `JiraErrorKind` (currently its own separate union) is updated too - `src/orchestrator/types/orchestrator.ts` — add `'network'` to `ErrorKind` (`error.json` schema) - `src/artifacts/helpers/validateErrorReport.ts` — add `'network'` to `VALID_KINDS` - `DESIGN.md` §3.4 — update the `error.json` kind enum documentation (currently `config|auth|not-found|rate-limit|transient|agent-failure|git|unknown`) ### 3. Wire classification into each request path In `makeAuthenticatedRequest()` (`src/shared/http/authenticatedFetch.ts`), wrap the `httpFetch(url, init)` call itself (not just the `!response.ok` branch) in a `try/catch` that runs the new `classifyFetchError` helper and throws the caller's `errorFactory`-equivalent for network errors — likely requires extending `AuthenticatedRequestOptions` with a `networkErrorFactory` (or reusing `errorFactory` with a sentinel status, e.g. `0`, mapped to `network` in `classifyHttpStatus`). Apply the same treatment to `JiraRestClient`'s separate `request()` method for consistency. ### 4. Fix `isRetriable` predicates to retry network errors Since `network` will be `retriable: true` in the shared `classifyHttpStatus`/kind policy, the existing `isRetriable: (error) => error instanceof MrClientError && error.retriable` predicates (and the `IssueSourceError`/`JiraError` equivalents) will automatically pick this up **once network failures are classified into those same error types** — no separate predicate change needed if step 3 is done correctly. Confirm this with a test that a network failure actually retries (per AGENTS.md's bounded-retry policy: 3 attempts, exponential backoff). ### 5. Fix the `classifyError()` gap for `MrClientError`/`IssueSourceError` Add explicit cases in `src/orchestrator/helpers/classifyError.ts` for `MrClientError` (mirroring the existing `JiraError`/`IssueSourceError` case: use `error.kind`/`error.retriable` directly) so MR-client failures of *any* kind (not just network) surface their real classification in `error.json` instead of the generic per-stage default. ## Test plan - **Unit — `classifyFetchError`**: recognizes `ENOTFOUND`, `ECONNREFUSED`, `ETIMEDOUT`, `ECONNRESET`, unknown `cause.code` (generic network fallback), non-`TypeError`/non-fetch errors (pass through unclassified). - **Unit — `classifyHttpStatus`**: `network` kind added, retriable `true`; existing status-code cases unaffected. - **Unit — `makeAuthenticatedRequest()`**: a mock `httpFetch` that throws a `TypeError('fetch failed')` (with a `cause`) is caught, classified, and retried up to `maxAttempts` per the injected retry options; asserts the final thrown error carries `kind: 'network'`, `retriable: true`, and an actionable message including the host and cause code. - **Unit — `JiraRestClient`'s `request()`**: same network-failure classification + retry behavior as `makeAuthenticatedRequest`. - **Unit — `classifyError()`**: new case for `MrClientError` mirroring the `JiraError`/`IssueSourceError` test — asserts `kind`/`retriable` pass through unchanged (covers both `network` and pre-existing HTTP-status kinds, closing the related gap in item 5). - **Unit — `validateErrorReport`**: `'network'` accepted as a valid `kind`. - **Regression — GitLab MR client with a real unreachable host**: integration-style test using the existing fake `httpFetch` seam, simulating a thrown `TypeError('fetch failed')` from `GitLabMrClient.createMr()`, asserting the surfaced error message names the unreachable host. ## Acceptance criteria - A REST call to a DNS-unresolvable or unreachable host produces an `error.json` with `kind: "network"`, `retriable: true`, and a `message` that names the host and the underlying cause (e.g. `ENOTFOUND`), not the bare string `"fetch failed"`. - Such failures are retried per the standard bounded-retry policy (3 attempts, exponential backoff) before failing the stage. - `MrClientError` failures of any kind (not just network) are classified correctly by `classifyError()` instead of falling back to the generic per-stage `agent-failure`/`unknown` default. - All existing HTTP-status-based classification (`401`/`403`/`404`/`429`/`5xx`) is unaffected — no regression in fail-fast vs retriable behavior for those cases. - `DESIGN.md` §3.4 and `error.json` schema/tests are updated to include `network` as a valid `kind`. ## Also consider (out of scope for this issue, noted for future work) - `src/mr/services/gitlabMergePoller.ts` / `githubMergePoller.ts` shell out to `glab`/`gh` CLIs via `execFile`, not `fetch` — a similarly opaque CLI-invocation failure (e.g. `glab` binary missing, or `glab` itself hitting a DNS failure) would need separate handling in `cliMergePollerBase.ts`'s `makeCliRunner` catch block, which currently just wraps `error.message` as `kind: 'unknown', retriable: false` regardless of cause. - No request-level timeout is currently configured on any `fetch` call (relies on OS-level TCP timeouts, which can be very slow for unreachable-but-routable hosts, e.g. black-holed IPs) — worth a separate issue to add `AbortSignal.timeout(...)` with a sensible default. ## Discovered while investigating Forgejo issue #265 (fixed): a *different*, unrelated resume-detection bug where the docs stage was needlessly re-run after a no-op completion, which was masking/compounding the diagnosis of this network-error issue in the same run.
david closed this issue 2026-08-18 07:05:08 +00:00
Author
Owner

pi-loop opened and merged a pull request for this issue: #272

pi-loop opened and merged a pull request for this issue: https://git.excelera.net/david/pi-loop/pulls/272
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
david/pi-loop#267
No description provided.