test(integration): retry early-hints deploy on transient npm/network errors#1511
Closed
dawsontoth wants to merge 2 commits into
Closed
test(integration): retry early-hints deploy on transient npm/network errors#1511dawsontoth wants to merge 2 commits into
dawsontoth wants to merge 2 commits into
Conversation
…errors deploy_component runs the component's `npm install`, which intermittently hits transient registry errors on CI (ECONNRESET / "Failed to install dependencies"), failing the early-hints suite in `before`. Retry the deploy up to 3x on transient install/network failures (with backoff) before giving up; on exhaustion, keep the existing behavior (log the full error, rely on the readiness poll). A single transient ECONNRESET no longer fails the suite; only a sustained outage does. Stacked on #1504. CI-validated (transient network errors aren't reproducible on demand; local restart_service is flaky). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a retry mechanism (up to 3 attempts) for component deployment in integration tests to handle transient network and installation errors. The review feedback suggests making the transient error detection more robust by checking both the message and code properties of the error object, as stringifying a plain object directly results in "[object Object]".
Contributor
|
Reviewed; no blockers found. |
Address review feedback (#1511): check the error's message, code, nested cause (message + code — undici fetch errors wrap the real ECONNRESET/timeout in `.cause`), and the stringified error, rather than only `message ?? e`. A code-only error, or one with an undefined message (where String(e) is "[object Object]"), would otherwise not be recognized as transient and wouldn't be retried. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Status: Draft — one of the focused follow-ups for the remaining shard-5/6 flakes. Stacked on #1504.
Flake
deploy_componentruns the early-hints component'snpm install, which on CI intermittently hits a transient registry error:When that happens, the deploy fails, the readiness poll then 404s for 60s, and the whole
early-hintssuite fails inbefore. (Same registry flake also took out a v22 build.) This is genuinely transient — the correct response is to retry.Fix
Retry the deploy up to 3× (with backoff) on transient install/network errors (
ECONNRESET/ETIMEDOUT/EAI_AGAIN/fetch failed/Failed to install dependencies). On exhaustion, keep the existing behavior from #1504 (log the full error, fall through to the readiness poll). So a single transientECONNRESETno longer fails the suite; only a sustained registry outage does.This composes with the #1504 deploy guard: the retry handles the transient-npm case; the guard still handles the slow-restart
HeadersTimeoutErrorcase.Validation
CI-only — transient network errors aren't reproducible on demand, and local
restart_serviceis flaky. The retry path is exercised whenever the registry hiccups.Scope note
This is the genuine-fix subset of the remaining flakes. Not addressed here (separate, by design): the
drop_databaseENOLCK (infra/engine — better as an investigation issue) and the terminology job-starvation timeout (band-aid over engine-side starvation). The HNSW reindex-tolerance fix is a separate PR stacked on #1505 (same file).🤖 Generated with Claude Code