Open
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
🦋 Changeset detectedLatest commit: 60b269e The changes in this PR will be included in the next version bump. This PR includes changesets to release 24 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
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughstartEnsDbWriterWorker became async and idempotent, re-reading shutdown managers at start; EnsDbWriterWorker run/stop now accept an AbortSignal, serialize in-flight snapshot upserts, and await stop; Ponder clients gain a dynamic abort-signal getter and runtime shutdown-manager validators were added. Changes
Sequence DiagramsequenceDiagram
participant Caller as startEnsDbWriterWorker
participant Context as LocalPonderContext
participant Shutdown as apiShutdown Manager
participant Singleton as EnsDbWriter Singleton
participant Worker as EnsDbWriterWorker
rect rgba(100,150,200,0.5)
Caller->>Context: getApiShutdown()
Context-->>Caller: validated apiShutdown
Caller->>Singleton: stop existing worker (if any)
Singleton-->>Caller: cleared
Caller->>Worker: new EnsDbWriterWorker()
Caller->>Shutdown: register callback -> stop this Worker
Caller->>Worker: run(abortSignal)
Worker->>Worker: schedule interval, respect signal and skip overlapping snapshots
end
rect rgba(200,100,100,0.5)
Shutdown->>Shutdown: abortController.signal aborted
Shutdown->>Singleton: invoke registered callback
Singleton->>Worker: await stop() (clear interval, await in-flight snapshot)
Worker-->>Singleton: stopped
Singleton->>Caller: worker reference cleared
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
tk-o
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to improve the local/dev experience for ENSIndexer by wiring Ponder runtime shutdown signals into the SDK/app so long-running processes and network requests can be aborted cleanly (resolving issue #1432).
Changes:
- Add an
abortSignaltoPonderAppContextand derive it from Ponder-provided shutdown controllers during deserialization. - Propagate the abort signal into
PonderClient/LocalPonderClientsofetch()requests can be canceled on shutdown. - Stop the ENSDb writer worker when the local Ponder app abort signal fires.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ponder-sdk/src/ponder-app-context.ts | Adds abortSignal to the app context interface for shutdown signaling. |
| packages/ponder-sdk/src/deserialize/ponder-app-context.ts | Deserializes new shutdown managers and builds a merged AbortSignal. |
| packages/ponder-sdk/src/client.ts | Adds optional abort signal support to fetch() calls. |
| packages/ponder-sdk/src/local-ponder-client.ts | Passes the app abort signal through to the base PonderClient. |
| apps/ensindexer/src/lib/ensdb-writer-worker/singleton.ts | Stops the ENSDb writer worker on abort to support clean shutdown in dev/serve. |
💡 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: 3
🤖 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/ensdb-writer-worker/singleton.ts`:
- Line 33: Update the logger.info call in singleton.ts so the startup message
matches the shutdown wording: change the string passed to logger.info (currently
"StartingEnsDbWriterWorker...") to "Starting EnsDbWriterWorker..." by editing
the logger.info invocation to include the missing space for consistency with the
shutdown log.
In `@packages/ponder-sdk/src/deserialize/ponder-app-context.ts`:
- Around line 60-71: The shutdown manager Zod schema
(schemaPonderAppShutdownManager) incorrectly constrains the callback signature
for add to return void; update the add entry so the inner callback's output
allows unknown or a Promise (e.g., use z.union([z.void(), z.unknown()]) for the
inner z.function output) so it accepts unknown | Promise<unknown>, or
alternatively relax the whole object like the logger schema by using
.passthrough()/.looseObject() to avoid overconstraining the callback signature;
ensure you still validate isKilled as z.boolean() and abortController as
z.instanceof(AbortController).
In `@packages/ponder-sdk/src/ponder-app-context.ts`:
- Around line 35-39: The mock object in local-ponder-client.mock.ts must include
the new abortSignal property to satisfy the PonderAppContext interface; update
the exported mock (the object asserted with "satisfies PonderAppContext") to add
abortSignal: new AbortController().signal (or reuse a shared AbortController if
needed) so TypeScript compilation succeeds and any long-running mock consumers
can be signalled for shutdown.
🪄 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: 8a3cd472-61a0-4a2d-8ea8-772f9879eba8
📒 Files selected for processing (5)
apps/ensindexer/src/lib/ensdb-writer-worker/singleton.tspackages/ponder-sdk/src/client.tspackages/ponder-sdk/src/deserialize/ponder-app-context.tspackages/ponder-sdk/src/local-ponder-client.tspackages/ponder-sdk/src/ponder-app-context.ts
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
shrugs
reviewed
Apr 15, 2026
Ponder's dev-mode hot reload re-executes the API entry file but caches transitively-imported modules, leaving module-level singleton state stale. On reload, startEnsDbWriterWorker() would throw "EnsDbWriterWorker has already been initialized" and kill the process. - local-ponder-context.ts is now a reactive proxy that re-reads globalThis.PONDER_COMMON.apiShutdown (and .shutdown) on every access. Ponder kills and replaces these managers on each reload (ponder/src/bin/commands/dev.ts:95-101), so cached references go stale immediately. - startEnsDbWriterWorker() is reset-aware: awaits any prior worker's stop(), registers cleanup via apiShutdown.add(), and ignores AbortError in the .run().catch() path so shutdown does not kill the process. - EnsDbWriterWorker.stop() is async and awaits any in-flight snapshot upsert, so Ponder's shutdown sequence can wait on it. - PonderClient/LocalPonderClient accept an optional getAbortSignal() getter, invoked at fetch time, so HTTP requests use the current signal instead of a captured-at-construct reference that goes stale per reload. Resolves #1432. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
shrugs
force-pushed
the
fix/1432-ensindexer-dev-mode-experience
branch
from
15cd750 to
73689cc
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/ensdb-writer-worker/ensdb-writer-worker.ts`:
- Around line 148-158: The tests call worker.stop() without awaiting its
now-async implementation; update each test invocation to use await worker.stop()
so the in-flight snapshot and interval cleanup complete before assertions finish
— specifically change the calls at the noted locations (the occurrences of
worker.stop() referenced in the comment) to await worker.stop() inside their
existing async test functions (mirror the pattern used in singleton.ts where
await worker.stop() is used).
🪄 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: 6ce727dd-cb8f-44ce-9691-2a5058c7b0dc
📒 Files selected for processing (6)
apps/ensindexer/src/lib/ensdb-writer-worker/ensdb-writer-worker.tsapps/ensindexer/src/lib/ensdb-writer-worker/singleton.tsapps/ensindexer/src/lib/local-ponder-client.tsapps/ensindexer/src/lib/local-ponder-context.tspackages/ponder-sdk/src/client.tspackages/ponder-sdk/src/local-ponder-client.ts
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
shrugs
changed the title
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
apps/ensindexer/src/lib/ensdb-writer-worker/singleton.ts:93
- This
worker.run().catch(...)chain is fire-and-forget, but the handler rethrows the error. Because the returned promise isn’t awaited/returned, that rethrow becomes an unhandled rejection (and may not reliably result in the intended shutdown behavior across Node settings). Either return/await therun()promise fromstartEnsDbWriterWorkerso initialization fails deterministically, or avoid rethrowing here and terminate explicitly (or mark as intentionally unawaited withvoid).
worker
.run()
// Handle any uncaught errors from the worker
.catch(async (error) => {
// If Ponder has begun shutting down our API instance (hot reload or
// graceful shutdown), the abort propagates through in-flight fetches
// as an AbortError. Treat that as a clean stop, not a worker failure.
if (apiShutdown.abortController.signal.aborted || isAbortError(error)) {
logger.info({
msg: "EnsDbWriterWorker stopped due to API shutdown",
module: "EnsDbWriterWorker",
});
return;
}
// Real worker error — clean up and trigger non-zero exit.
await worker.stop();
if (ensDbWriterWorker === worker) {
ensDbWriterWorker = undefined;
}
logger.error({
msg: "EnsDbWriterWorker encountered an error",
error,
});
// Re-throw the error to ensure the application shuts down with a non-zero exit code.
process.exitCode = 1;
throw error;
});
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- await worker.stop() in all ensdb-writer-worker tests (stop() is async) - skip overlapping snapshot upserts via inFlightSnapshot guard so a slow upsert can't pile up concurrent ENSDb writes; stop() awaits the single in-flight upsert deterministically - thread an optional AbortSignal into worker.run() and check between await steps so a hot reload during run() startup bails before the recurring interval is scheduled (closes the startup race) - extract gracefulShutdown helper in singleton.ts shared by the apiShutdown.add() callback and the .run().catch() paths - add PonderClient unit tests asserting the getAbortSignal getter is invoked per-fetch, returns fresh identity across calls, and cancels in-flight fetches when aborted Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… path When the defensive stale-instance cleanup inside startEnsDbWriterWorker calls worker.stop() on the previous worker, the old run() throws an AbortError via the new stopRequested guard — but the captured abortSignal may not be aborted (the safety net runs precisely when Ponder's own apiShutdown.add() callback didn't fire). The catch path now also treats `ensDbWriterWorker !== worker` as a clean-stop signal, so the supersession path doesn't get misclassified as a fatal error and call process.exit(1). isAbortError() is still required so unrelated failures are not silently swallowed. Surfaced by copilot review. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Collaborator
|
@greptileai please re-review — addressed copilot's catch-path classification: defensive stale-instance cleanup now correctly handles AbortError from the new stopRequested guard via an |
…ellation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…getters The Proxy was technically correct but call-site ergonomics hid the staleness contract: `localPonderContext.apiShutdown` looked like an ordinary property access, masking that it was actually a fresh read from globalThis. A captured `const sig = localPonderContext.apiShutdown .signal` looked innocent but was a footgun. Split the context into two shapes: - `localPonderContext` is back to an eager `const` for the stable fields (command, localPonderAppUrl, logger). Ponder doesn't mutate these, and the original code shape already matched this. - Reload-scoped fields are now plain functions: `getApiShutdown()` and `getShutdown()`. The function-call form makes it visible at every call site that the value is freshly read each call. A captured `getApiShutdown().signal` obviously caches a function result, which reads as a bug. Drops the Proxy, the symbol-prop guard, the LocalPonderContext interface, the prop-as-keyof cast, and the cachedStableContext memoization helper. Net: 19 fewer lines, simpler shape, better ergonomics. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Collaborator
|
@greptileai please re-review — refactored localPonderContext from a Proxy to explicit |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…on tests - In gracefulShutdown(), clear ensDbWriterWorker before awaiting worker.stop() so the catch discriminator immediately sees the worker as superseded. Prevents misclassifying a stop-driven AbortError as fatal when the run().catch fires before the singleton was cleared. - Add test: stop() during run() startup rejects with AbortError and never arms the recurring interval. - Add test: overlapping snapshot ticks are skipped when a prior upsert is still in flight, and resume once it settles. Co-Authored-By: Claude Opus 4.6 (1M context) <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
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.
fix(ensindexer): hot-reload safe writer worker
Reviewer Focus
apps/ensindexer/src/lib/local-ponder-context.ts— thegetApiShutdown()/getShutdown()function pattern. The staleness contract (always read fresh, never cache) is enforced ergonomically by making call sites function calls rather than property accesses.apps/ensindexer/src/lib/ensdb-writer-worker/singleton.ts— the.run().catch()clean-stop discrimination:(abortSignal.aborted || ensDbWriterWorker !== worker) && isAbortError(error). Both abort-source paths must be validated to avoid masking real errors AND to avoid misclassifying intentional supersession as a fatal error.apps/ensindexer/src/lib/ensdb-writer-worker/ensdb-writer-worker.ts— thestopRequestedguard.stop()flips it;run()checks viacheckCancellation()between every async setup step so a stop during startup actually cancels.Problem
ponder dev-mode hot reload of indexing handler files crashed ensindexer with
Error: EnsDbWriterWorker has already been initializedandprocess.exit(1).Root cause: ponder kills and replaces
common.shutdownandcommon.apiShutdownon every indexing reload (ponder/src/bin/commands/dev.ts:95-101), then re-execs the api entry file. but vite-node only invalidates the changed file's dep tree — api-side singleton modules stay cached. so module-level state survives across reloads (singleton check throws), and any reference cached during the first boot goes stale.resolves #1432.
What Changed
local-ponder-context.ts— stable fields stay as an eagerconst. reload-scoped fields are exposed as functionsgetApiShutdown()/getShutdown()that re-readglobalThis.PONDER_COMMONon every call. the function-call shape makes the fresh-read contract visible at every call site, so accidental caching reads as obviously wrong (const sig = getApiShutdown().signalclearly caches a function result; the equivalent property access wouldn't).singleton.ts—startEnsDbWriterWorker()is reset-aware. drops the throw-on-re-init, awaits any prior worker'sstop(), registers cleanup viagetApiShutdown().add(...)so ponder properly awaits worker shutdown during its kill sequence. extractedgracefulShutdown(worker, reason)helper..run().catch()distinguishes intentional stops (signal aborted OR worker superseded in singleton, AND error isAbortError) from real failures, which fail-fast viaprocess.exit(1).ensdb-writer-worker.ts—stop()is async.inFlightSnapshottracks the latest upsert with skip-overlap sostop()deterministically awaits one promise. newstopRequestedflag +checkCancellation()helper letstop()cancel an in-progressrun()startup before it arms the recurring interval.run()accepts an optional externalAbortSignalfor the same purpose.packages/ponder-sdk—PonderClientaccepts an optionalAbortSignalGetter(typed alias).LocalPonderClientforwards it. invoked at fetch time insidehealth/metrics/statusso requests use the current signal instead of a captured-at-construct reference.PonderAppShutdownManagerinterface +isPonderAppShutdownManagerguard exported alongsidePonderAppContextsince they mirror a Ponder runtime type.Self-Review
globalThis.PONDER_COMMONlost when read moved into a function (added explicit check);isAbortErrororiginally only checkedinstanceof Errorand missedDOMException(the actual fetch-abort rejection type); the catch-path discriminator went through||→&&→(signal.aborted || superseded) && isAbortErroras the supersession case from the defensive cleanup path was identified.ProxyoverglobalThis.PONDER_COMMONwith explicit getter functions. the proxy was technically correct but hid the staleness contract behind property-access syntax. -19 lines.typeof ensDbWriterWorker !== "undefined"throw is gone — re-init is the expected path.Cross-Codebase Alignment
ensDbClient,ensRainbowClient,publicConfigBuilder,indexingStatusBuilder,localPonderClient) for stale-reference hazards. onlylocalPonderClientcaptured a runtime mutable (the abort signal), addressed via the getter-pattern threading.publicConfigBuilderandindexingStatusBuildercacheimmutablePublicConfig/_immutableIndexingConfigafter first build. could mask config changes if you editconfig.tsand reload — pre-existing pre-fix behavior, not the crash, deferred.Downstream & Consumer Impact
PonderClientconstructor: optional second arggetAbortSignal?: AbortSignalGetter. backwards compatible.LocalPonderClientconstructor: optional 5th arg, same. backwards compatible.EnsDbWriterWorker.stop()is nowasyncand returnsPromise<void>. existing call sites (production + tests) updated to await.@ensnode/ponder-sdk:AbortSignalGetter,PonderAppShutdownManager,isPonderAppShutdownManager.Testing
pnpm -F ensindexer typecheck✓pnpm -F @ensnode/ponder-sdk typecheck✓pnpm test --project ensindexer --project @ensnode/ponder-sdk→ 268 passed (4 newPonderClienttests for the getAbortSignal getter pattern).pnpm lint✓pnpm -F ensindexer devagainst ens-test-env, edited an indexing handler multiple times. confirmed: hot reload completes, no "already been initialized", noELIFECYCLE, indexing resumes, snapshots continue upserting.no automated hot-reload test infrastructure exists; manual smoke test is the only path.
Risk Analysis
(signal.aborted || ensDbWriterWorker !== worker) && isAbortError(error)— both an abort-source signal AND the AbortError name are required, ruling out both "real error during shutdown" and "AbortError from a cross-contamination race that didn't actually stop us."stopRequested+ the externalAbortSignalprovide two independent cancellation channels.checkCancellation()honors both.process.exit(1)on real worker errors is explicit and fail-fast. can't rethrow from the fire-and-forget catch (would become an unhandled rejection rather than reaching api/index.ts).Pre-Review Checklist