Open
Conversation
🦋 Changeset detectedLatest commit: bbe0ccc The changes in this PR will be included in the next version bump. This PR includes changesets to release 19 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces a retry mechanism with exponential backoff (up to 3 attempts, 1–30s timeout) for ENSRainbow API heal() calls during indexing. A new wrapper class delegates most operations directly but wraps heal() with retry logic using p-retry. Type signatures are updated across the codebase from Changes
Sequence DiagramsequenceDiagram
participant Indexer as Indexing Process
participant Wrapper as Retry Wrapper
participant Retry as p-retry
participant Client as ENSRainbow Client
participant API as ENSRainbow API
Indexer->>Wrapper: heal(labelHash)
Wrapper->>Retry: execute with retry config
loop Retry Loop (up to 3 attempts)
Retry->>Client: heal(labelHash)
Client->>API: HTTP request
alt Success (HealSuccess)
API-->>Client: label resolved
Client-->>Retry: return LiteralLabel
Retry-->>Wrapper: success
else Transient Error (HealServerError / Network)
API-->>Client: error
Client-->>Retry: throw error
Retry->>Wrapper: log warning, retry
else Non-Transient (404 / 400)
API-->>Client: error
Client-->>Retry: throw HealNotFoundError or HealBadRequestError
Retry-->>Wrapper: no retry, fail immediately
end
end
Wrapper-->>Indexer: result or throw non-recoverable error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment Tip CodeRabbit can scan for known vulnerabilities in your dependencies using OSV Scanner.OSV Scanner will automatically detect and report security vulnerabilities in your project's dependencies. No additional configuration is required. |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a Cursor “plan” document describing the intended approach for implementing an ENSRainbow API client retry wrapper in ensindexer (issue #214).
Changes:
- Introduces a new plan file outlining retry-wrapper design, wiring, and test strategy.
- Documents current ensindexer call paths (startup/config fetch and heal path) and why long-duration retries are needed.
- Proposes using
p-retrywith warning logs on failed attempts.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.cursor/plans/ensrainbow_client_retry_wrapper_e7106d29.plan.md:
- Line 67: The wrapper cannot be a plain object typed as EnsRainbowApiClient if
EnsRainbowApiClient is a concrete class; update the plan and implementation by
either (a) extracting/defining an interface (e.g. IEnsRainbowClient) that
declares the public methods used (heal, config, getOptions, optional health) and
type the wrapper and factory to return that interface, (b) make the wrapper
actually extend EnsRainbowApiClient so it is an instance of the class and can be
returned as EnsRainbowApiClient, or (c) keep structural typing but ensure the
wrapper implements all methods and change the factory return type to the
interface/union to reflect compatibility; update references to
EnsRainbowApiClient and the factory return type accordingly so consumers remain
type-safe.
- Around line 39-44: Update the plan to explicitly recommend making retry timing
configurable via environment variables rather than fixed defaults: propose
specific env var names (ENSRAINBOW_RETRY_MAX_WAIT_MS,
ENSRAINBOW_RETRY_MIN_TIMEOUT_MS, ENSRAINBOW_RETRY_ATTEMPTS) with suggested
defaults (e.g. ~20min max wait, min timeout 30_000–60_000 ms, 40 attempts), note
that these defaults can be tuned per environment (dev/staging/prod), and include
a short caution to document how these values interact with the worker's existing
p-retry/backoff behavior so operators can adjust without code changes.
- Around line 77-81: Add a test that simulates the long-duration startup retry
by exercising the ENSRainbow retry wrapper (reference ENSRainbowClientWithRetry
or the factory getENSRainbowApiClient) to ensure it tolerates many consecutive
failures before succeeding; make the wrapper's backoff/delay configurable
(expose a short test-specific backoff via constructor params or env) so tests
can mock timers and advance time (use fake timers to simulate e.g. 10 failed
attempts then success without waiting 20 minutes), assert number of attempts,
that console.warn is called on each failure, and that final success returns the
result; also document/update how existing tests mock getENSRainbowApiClient to
either return a mock client behind the wrapper or mock the factory so the
wrapper is not exercised.
- Around line 23-27: The review points out nested retries between the
EnsDbWriterWorker's p-retry around getValidatedEnsIndexerPublicConfig (which
calls publicConfigBuilder.getPublicConfig -> ensRainbowClient.config) and the
new retry wrapper; resolve by choosing one of the approaches: (A) make the
wrapper detect "startup mode" (via an env flag or a parameter passed from
startEnsDbWriterWorker) and only apply long-duration retry there while using
short retries for runtime heal()/config(); or (B) remove/reduce the worker's
p-retry and let the wrapper own all retry behavior (adjusting wrapper retry
counts/backoff accordingly); implement the chosen approach by updating
startEnsDbWriterWorker/startup call site to pass the mode or by modifying
EnsDbWriterWorker's retry configuration so getValidatedEnsIndexerPublicConfig no
longer nests retries with ensRainbowClient.config, and document the behavior
change.
- Line 42: The ordered list formatting in the markdown uses explicit incremental
numbers (e.g., "2. **Health endpoint**") which violates the project's
auto-numbering style; update all ordered list items in this file (e.g., the line
containing "2. **Health endpoint**") to use "1." for every list item so the list
uses Markdown's automatic numbering convention.
- Around line 42-44: The retry wrapper currently considers wrapping client
methods indiscriminately; exclude the health() method from the retry policy (or
make per-method policies configurable) so health() calls remain fast-failing for
orchestration, while keeping heal() and config() wrapped with the existing
retry/backoff; update the retry wrapper (ensrainbow_client_retry_wrapper / retry
decorator) to check the method name (health) and either bypass retries or apply
a separate no/low-retry policy, and expose a simple per-method config map so
callers can override behavior if needed.
- Around line 45-48: Clarify and reconcile retry semantics between functions:
explicitly state that heal() returns discriminated union errors
(HealServerError, HealNotFoundError, HealBadRequestError) and does not throw on
HTTP error responses while config() throws on !response.ok, and either (a)
update the retry wrapper to treat returned HealServerError (and any
non-cacheable marker / server-5xx-like errorCode) as retryable by inspecting
heal()’s response object, or (b) document that heal() errors are non-retryable
and adjust the plan/questions to call out this thrown-vs-returned distinction
and justify which error classes (HealServerError vs
HealBadRequestError/HealNotFoundError) should be retried. Ensure you reference
heal(), config(), HealServerError/HealNotFoundError/HealBadRequestError and the
non-cacheable marker when updating the text or implementing the wrapper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a64265fb-17d8-4a2a-b731-0e9e196e94f8
📒 Files selected for processing (1)
.cursor/plans/ensrainbow_client_retry_wrapper_e7106d29.plan.md
…lures during indexing. Enhance overview to clarify retry mechanism for `heal()` with backoff and logging, while ensuring `config()` remains a plain delegation. Introduce a wrapper class for `heal()` with retry logic using p-retry, allowing for long-duration waits at startup. Adjust retry policy parameters for better control.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.cursor/plans/ensrainbow_client_retry_wrapper_e7106d29.plan.md (1)
22-22:⚠️ Potential issue | 🟡 MinorAlign this sentence with the later
heal()error semantics.Line 22 still says network failures can “return
HealServerError”, but the later section correctly says network/fetch failures throw and HTTP server failures are returned asHealServerError. Keeping both versions in the plan is confusing.Suggested fix
-The SDK uses `fetch()`; network failures cause `heal()` to throw (or return HealServerError). Applying the wrapper at the factory ensures `heal()` gets retries without touching handlers. +The SDK uses `fetch()`; network/fetch failures cause `heal()` to throw, while HTTP server failures are returned as `HealServerError`. Applying the wrapper at the factory ensures `heal()` gets retries without touching handlers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.cursor/plans/ensrainbow_client_retry_wrapper_e7106d29.plan.md at line 22, Update the sentence to match the later semantics: state that fetch()/network failures cause heal() to throw (not return) an exception while HTTP server failures are returned as a HealServerError; mention applying the wrapper at the factory ensures heal() gets retries without changing handlers. Reference heal(), HealServerError, fetch(), and the factory/wrapper so the author knows which concepts to edit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.cursor/plans/ensrainbow_client_retry_wrapper_e7106d29.plan.md:
- Around line 18-20: In the plan file
.cursor/plans/ensrainbow_client_retry_wrapper_e7106d29.plan.md the repo-relative
Markdown links like "[apps/ensindexer/src/lib/ensraibow-api-client.ts]" and
other "[apps/...]" entries are resolving incorrectly from the .cursor/plans
directory; update those link targets to be rebased one level up (change
"apps/..." to "../../apps/...") throughout the file so they point to the correct
repository paths (apply the same "../../" prefix to all subsequent links that
start with "apps/").
---
Duplicate comments:
In @.cursor/plans/ensrainbow_client_retry_wrapper_e7106d29.plan.md:
- Line 22: Update the sentence to match the later semantics: state that
fetch()/network failures cause heal() to throw (not return) an exception while
HTTP server failures are returned as a HealServerError; mention applying the
wrapper at the factory ensures heal() gets retries without changing handlers.
Reference heal(), HealServerError, fetch(), and the factory/wrapper so the
author knows which concepts to edit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 55ab0f22-9ef6-4de5-8920-f23145c5c38a
📒 Files selected for processing (1)
.cursor/plans/ensrainbow_client_retry_wrapper_e7106d29.plan.md
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: lightwalker.eth <126201998+lightwalker-eth@users.noreply.github.com>
Implement a new `EnsRainbowClientWithRetry` class to handle transient failures in `heal()` with exponential backoff and logging. Update the `getENSRainbowApiClient` function to return this new client. Add tests to ensure retry behavior works as expected, including handling various error scenarios and logging warnings on failed attempts.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/ensindexer/src/lib/ensrainbow-client-with-retry.ts`:
- Around line 43-76: Add a short inline comment above the catch block (around
the return of lastServerError in heal) explaining the deliberate asymmetric
error handling: that when the inner call returns a HealServerError (detected by
isHealError and ErrorCode.ServerError) we store and return that error after
retries to preserve API compatibility with callers like labelByLabelHash (in
graphnode-helpers.ts) that expect heal to resolve with a HealServerError value,
whereas actual network/fetch exceptions are rethrown after retry exhaustion;
reference heal and lastServerError in the comment to make intent clear to future
maintainers.
In `@packages/integration-test-env/src/orchestrator.ts`:
- Around line 349-355: The execaSync invocation currently sets env to only
ENSNODE_URL, which omits inheriting process.env (so PATH and other vars are
lost); update the execaSync call that runs pnpm test:integration to merge the
current environment and override ENSNODE_URL (e.g. env: { ...process.env,
ENSNODE_URL: `http://localhost:${ENSAPI_PORT}` }) — mirror the same fix used in
spawnService so the test runner gets PATH and other required env vars while
still passing ENSNODE_URL to the process.
- Around line 156-164: The spawn call is replacing the whole environment instead
of inheriting parent env; update spawnService to merge process.env with the
provided env before passing to spawn (e.g., const mergedEnv = { ...process.env,
...env } and pass mergedEnv), so subprocess created by spawn (the subprocess
variable) retains PATH, HOME, etc., while still allowing overrides from the
provided env.
- Around line 188-191: The dynamic import of EnsIndexerClient inside
pollIndexingStatus causes inconsistent imports; add EnsIndexerClient to the
existing static import alongside OmnichainIndexingStatusIds from
"@ensnode/ensnode-sdk", then replace the dynamic new (await
import(...)).EnsIndexerClient usage in pollIndexingStatus with a direct new
EnsIndexerClient({ url: new URL(ENSINDEXER_URL) }) to keep imports consistent
and improve startup performance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a6852a3a-1659-4aa0-a5b6-5392fc19a9f9
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (29)
.changeset/fluffy-hairs-draw.md.changeset/purple-clubs-strive.md.changeset/strong-baboons-help.md.github/workflows/test_ci.ymlapps/ensapi/src/graphql-api/schema/domain.tsapps/ensapi/src/graphql-api/schema/query.integration.test.tsapps/ensapi/src/handlers/ensnode-graphql-api.tsapps/ensapi/src/handlers/subgraph-api.tsapps/ensapi/src/middleware/require-core-plugin.middleware.tsapps/ensapi/src/test/integration/ensnode-graphql-api-client.tsapps/ensapi/src/test/integration/global-setup.tsapps/ensindexer/ponder/ponder.config.tsapps/ensindexer/src/config/validations.tsapps/ensindexer/src/lib/ensraibow-api-client.tsapps/ensindexer/src/lib/ensrainbow-client-with-retry.test.tsapps/ensindexer/src/lib/ensrainbow-client-with-retry.tsapps/ensindexer/src/lib/public-config-builder/public-config-builder.test.tsapps/ensindexer/src/lib/public-config-builder/public-config-builder.tsapps/ensindexer/src/plugins/ensv2/handlers/ensv1/ENSv1Registry.tsapps/ensindexer/src/plugins/index.tsapps/ensrainbow/scripts/download-prebuilt-database.shpackage.jsonpackages/ensnode-schema/src/schemas/ensv2.schema.tspackages/ensnode-schema/src/schemas/protocol-acceleration.schema.tspackages/ensnode-sdk/src/graphql-api/example-queries.tspackages/integration-test-env/LICENSEpackages/integration-test-env/README.mdpackages/integration-test-env/package.jsonpackages/integration-test-env/src/orchestrator.ts
💤 Files with no reviewable changes (2)
- apps/ensapi/src/middleware/require-core-plugin.middleware.ts
- apps/ensindexer/src/plugins/index.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/ensindexer/src/lib/ensraibow-api-client.ts (1)
5-20:⚠️ Potential issue | 🟠 MajorWire retry handling transparently into this factory or delegate explicitly.
The factory returns a raw
EnsRainbowApiClientwithout retry wrapping. WhilelabelByLabelHash()ingraphnode-helpers.tswraps the specificheal()call withpRetry(), this retry logic is not centralized at the factory boundary. Other call sites likepublic-config-builder/singleton.ts:4receive the raw client and have no retry handling. Either return a wrapped client from the factory that handles retries transparently, or document that callers are responsible for implementing retry logic where needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ensindexer/src/lib/ensraibow-api-client.ts` around lines 5 - 20, The factory getENSRainbowApiClient currently returns a raw EnsRainbowApiClient without centralized retry behavior; update getENSRainbowApiClient to return a client wrapper that transparently retries network/heal calls (use the same pRetry options used in labelByLabelHash or a shared retry policy) so callers (e.g., public-config-builder/singleton.ts) don't need to implement retries; implement the wrapper around EnsRainbowApiClient methods that perform network requests (or decorate EnsRainbowApiClient.heal/labelByLabelHash-equivalents) and ensure the wrapper delegates successful responses and surfaces errors after retry exhaustion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/ensindexer/src/lib/graphnode-helpers.test.ts`:
- Around line 152-169: Add a global cleanup so spies are always restored: remove
per-test warnSpy.mockRestore() calls and add an afterEach that calls
vi.restoreAllMocks() (or vi.resetAllMocks() as appropriate) alongside existing
beforeEach clearing; update tests referencing labelByLabelHash to rely on the
centralized afterEach cleanup to avoid leaking the console.warn spy across the
four test cases.
---
Outside diff comments:
In `@apps/ensindexer/src/lib/ensraibow-api-client.ts`:
- Around line 5-20: The factory getENSRainbowApiClient currently returns a raw
EnsRainbowApiClient without centralized retry behavior; update
getENSRainbowApiClient to return a client wrapper that transparently retries
network/heal calls (use the same pRetry options used in labelByLabelHash or a
shared retry policy) so callers (e.g., public-config-builder/singleton.ts) don't
need to implement retries; implement the wrapper around EnsRainbowApiClient
methods that perform network requests (or decorate
EnsRainbowApiClient.heal/labelByLabelHash-equivalents) and ensure the wrapper
delegates successful responses and surfaces errors after retry exhaustion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 86fef2d2-0f3c-4a6f-ad82-d124201605fe
📒 Files selected for processing (3)
apps/ensindexer/src/lib/ensraibow-api-client.tsapps/ensindexer/src/lib/graphnode-helpers.test.tsapps/ensindexer/src/lib/graphnode-helpers.ts
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
…mocks and improve error handling in retry logic. Update error throwing to include cause for better debugging.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
apps/ensindexer/src/lib/graphnode-helpers.ts:86
- The network-error exhaustion path rethrows the original error after mutating
error.message, but does not wrap/attach the underlying failure ascause. This contradicts the PR description (“attach the last thrown error as cause when retries exhaust”) and also drops useful context (original message) for callers; consider throwing a newErrorwith a descriptive message (include endpoint + labelHash) and set{ cause: error }(similar to apps/ensindexer/src/lib/ensdb-writer-worker/ensdb-writer-worker.ts:177-180) instead of mutating and rethrowing the original error.
// Not recoverable; causes the ENSIndexer process to terminate.
if (error instanceof Error) {
error.message = `ENSRainbow Heal Request Failed: ENSRainbow unavailable at '${ensRainbowApiClient.getOptions().endpointUrl}'.`;
}
throw error;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
…ryable errors from ENSRainbow and clarify transient error conditions.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
apps/ensindexer/src/lib/graphnode-helpers.ts:87
- When retries are exhausted due to a thrown error (e.g. persistent network/fetch failures), this path mutates and rethrows the original error without attaching a
cause, even though the PR description says the last error should be preserved ascausefor debuggability. Consider throwing a new Error for the “unavailable” case as well (withcauseset to the last retry error) instead of overwritingerror.message, so callers/logs can retain both the high-level context and the underlying failure details.
// Not recoverable; causes the ENSIndexer process to terminate.
if (error instanceof Error) {
error.message = `ENSRainbow Heal Request Failed: ENSRainbow unavailable at '${ensRainbowApiClient.getOptions().endpointUrl}'.`;
}
throw error;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
lightwalker-eth
approved these changes
Mar 15, 2026
Member
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@djstrong Thank you. Looks good 👍 Shared a few very small suggestions please take the lead to merge when ready 👍
| * - Network/fetch failure: heal() throws because the ENSRainbow service was unreachable. | ||
| * - HealServerError (errorCode 500): ENSRainbow returned a transient server-side error. | ||
| * | ||
| * Non-transient outcomes are handled immediately without retry, because retrying would not change |
Member
There was a problem hiding this comment.
Suggested change
| * Non-transient outcomes are handled immediately without retry, because retrying would not change | |
| * Non-transient outcomes are thrown immediately without retry, because retrying would not change |
| * transient failure (e.g. a momentary network blip or a brief ENSRainbow server hiccup) would | ||
| * otherwise crash the ENSIndexer process, forcing a full restart and re-index from the last | ||
| * checkpoint. To avoid this disproportionate impact, transient failures are retried with | ||
| * exponential backoff (up to 3 retries, with a 1–30 second timeout range) before the error is surfaced: |
Member
There was a problem hiding this comment.
Suggested change
| * exponential backoff (up to 3 retries, with a 1–30 second timeout range) before the error is surfaced: | |
| * exponential backoff (up to 3 retries, with a 1–30 second timeout range) before the error is thrown: |
| * | ||
| * ## Non-recoverable throws | ||
| * | ||
| * Any throw from this function is not recoverable. It propagates to the calling indexing handler |
Member
There was a problem hiding this comment.
Suggested change
| * Any throw from this function is not recoverable. It propagates to the calling indexing handler | |
| * Any throw from this function is not recoverable or has exceeded the max retries. It propagates to the calling indexing handler |
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
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.
Lite PR
Tip: Review docs on the ENSNode PR process
Summary
labelByLabelHash(graphnode-helpers) for ENSRainbowheal()calls during indexing, using p-retry: retries on network errors and transientHealServerError(500) responses (up to 3 retries, 1–30s timeout range).Errorwith a descriptive message and attach the last thrown error ascauseso logs and stack traces retain the underlying failure and retry context.graphnode-helpers.test.ts: addafterEach(() => vi.restoreAllMocks())in the retry-behavior describe and remove per-testwarnSpy.mockRestore()so theconsole.warnspy does not leak across tests.Why
causeimproves debuggability when retries are exhausted.Testing
graphnode-helpers.test.ts: 14 tests, including a “retry behavior” describe with tests for: retry on network/fetch failure then success, retry onHealServerErrorthen success, no retry onHealNotFoundError/HealBadRequestError, exhaustion on persistent network failures, exhaustion on persistentHealServerError(with assertion that the thrown error has the expectedcause). Spy cleanup is done inafterEachwithvi.restoreAllMocks().Notes for Reviewer (Optional)
heal()path used bylabelByLabelHashis retried; other ENSRainbow client usage (e.g.config/health/count/version) is unchanged.HealServerError), the function throws. ForHealServerErrorexhaustion we throw a newErrorwith a structured message and the last p-retry error ascause(not a returnedHealServerError), so the API contract remains “throw on unrecoverable failure” and logs keep full context.retries: 3,minTimeout: 1_000,maxTimeout: 30_000), not env variables.Pre-Review Checklist (Blocking)