Use Ponder runtime logger across ENSIndexer app#1864
Merged
Conversation
🦋 Changeset detectedLatest commit: c6b7cf7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 23 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. 3 Skipped Deployments
|
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:
📝 WalkthroughWalkthroughAdds a structured Ponder logger API and Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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)
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 |
1 task
Contributor
There was a problem hiding this comment.
Pull request overview
This PR standardizes logging across the ENSIndexer app by routing logs through Ponder’s runtime logger, and updates @ensnode/ponder-sdk’s app context model to expose that logger consistently.
Changes:
- Extend
PonderAppContextto include a runtimeloggerand validate it during deserialization. - Add
apps/ensindexer/src/lib/logger.ts(runtime logger +formatLogParam) and replaceconsole.*usage with structured logger calls across ENSIndexer. - Update Vitest tests to mock the logger module and assert against
logger.*calls.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ponder-sdk/src/ponder-app-context.ts | Adds PonderAppLogger and logger field on PonderAppContext. |
| packages/ponder-sdk/src/local-ponder-client.mock.ts | Updates mock context to include a no-op logger. |
| packages/ponder-sdk/src/deserialize/ponder-app-context.ts | Validates/deserializes logger from PONDER_COMMON into PonderAppContext. |
| apps/ensindexer/src/lib/logger.ts | New logger wrapper pulling from globalThis.PONDER_COMMON.logger + formatLogParam. |
| apps/ensindexer/src/lib/test/mockLogger.ts | New helper to mock @/lib/logger in Vitest. |
| apps/ensindexer/src/plugins/ensv2/handlers/ensv1/NameWrapper.ts | Replaces a console.warn with logger.warn. |
| apps/ensindexer/src/lib/version-info.ts | Replaces console.warn with structured logger.error on lookup failure. |
| apps/ensindexer/src/lib/graphnode-helpers.ts | Replaces retry warning logs with structured logger.warn. |
| apps/ensindexer/src/lib/graphnode-helpers.test.ts | Updates tests to spy on logger.warn instead of console.warn. |
| apps/ensindexer/src/lib/ensraibow-api-client.ts | Replaces console.warn with structured warning about default ENSRainbow endpoint. |
| apps/ensindexer/src/lib/ensdb/migrate-ensnode-schema.ts | Replaces console.log with logger.debug/info around migrations. |
| apps/ensindexer/src/lib/ensdb-writer-worker/singleton.ts | Replaces console.error with logger.error for worker failures. |
| apps/ensindexer/src/lib/ensdb-writer-worker/ensdb-writer-worker.ts | Replaces multiple console.* calls with structured logger calls across worker lifecycle. |
| apps/ensindexer/src/lib/ensdb-writer-worker/ensdb-writer-worker.test.ts | Ensures logger is mocked for worker tests. |
| apps/ensindexer/src/lib/dns-helpers.ts | Replaces console.warn with structured logger.warn in DNS parsing helpers. |
| apps/ensindexer/src/lib/dns-helpers.test.ts | Ensures logger is mocked for DNS helper tests. |
| apps/ensindexer/ponder/src/api/index.ts | Uses runtime logger for init and API error handling. |
| apps/ensindexer/ponder/src/api/handlers/ensnode-api.ts | Uses runtime logger for API error handling. |
| apps/ensindexer/ponder/ponder.config.ts | Logs startup config and plugin warnings via runtime logger. |
| .changeset/warm-geese-fall.md | Changeset for @ensnode/ponder-sdk context update. |
| .changeset/cuddly-steaks-jog.md | Changeset for ENSIndexer logging enhancement. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/ensindexer/src/lib/ensdb-writer-worker/ensdb-writer-worker.ts
Outdated
Show resolved
Hide resolved
Contributor
Greptile SummaryThis PR replaces all
The structural design is clean and the abstraction layers are well-separated. The wrapper's intentional behavior of silently dropping non- Confidence Score: 5/5Safe to merge — all remaining findings are P2 style suggestions that do not affect correctness or runtime behavior All prior P1 concerns from earlier review rounds have been acknowledged and deliberately resolved (error-dropped warnings, log-level escalation). The new logger infrastructure is well-typed, correctly pipelines through Zod validation, and the wrapLogMethod silent-filter behavior is intentional and documented. The only new findings are a misplaced import in a test file and a redundant/misleading comment in mockLogger.ts — neither blocks merge. No files require special attention; minor style notes on apps/ensindexer/src/lib/graphnode-helpers.test.ts and apps/ensindexer/src/lib/test/mockLogger.ts Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Ponder runtime global] -->|injected at startup| B[local-ponder-context.ts
deserializePonderAppContext]
B --> C[Validate raw logger shape
via Zod schema]
C --> D[wrapPonderAppLogger
auto-formats log params]
D --> E[localPonderContext.logger]
E --> F[lib/logger.ts
singleton export]
F --> G[ponder.config.ts]
F --> H[api/index.ts + ensnode-api.ts]
F --> J[ensdb-writer-worker.ts]
F --> K[ensrainbow/singleton.ts]
F --> L[dns-helpers.ts, graphnode-helpers.ts, NameWrapper.ts, version-info.ts]
D --> M{wrapLogMethod per call}
M -->|string/number/boolean/Error| N[pass through unchanged]
M -->|plain object| O[JSON.stringify compact
URL to href, Map to obj, Set to arr]
M -->|error field with non-Error value| P[filtered out silently]
Reviews (4): Last reviewed commit: "Apply AI PR feedback" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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 @.changeset/cuddly-steaks-jog.md:
- Line 5: The changeset description contains a typo: replace the word "apporach"
with "approach" in the .changeset/cuddly-steaks-jog.md changeset description so
the release note reads "Enhanced application logging approach to use a
streamlined logger implementation across ENSIndexer app." Ensure only the
misspelled word is corrected and the rest of the sentence remains unchanged.
In `@apps/ensindexer/src/lib/dns-helpers.ts`:
- Line 171: The warning messages in parseDnsTxtRecordArgs use inconsistent
function labels and unstructured payloads: replace the hardcoded prefix
"parseDNSRecordArgs" with the correct function name parseDnsTxtRecordArgs and
pass a structured single-line payload using formatLogParam(txtDatas) instead of
pre-joining answers/strings; update all logger.warn calls in that function
(including the one currently logging `No TXT answers found...` and the
neighboring warnings at lines ~178-181) to use logger.warn({ msg:
'parseDnsTxtRecordArgs: ...', ...formatLogParam(txtDatas) }) or equivalent
structured payload so logs are consistent and searchable.
In `@apps/ensindexer/src/lib/logger.ts`:
- Around line 18-23: The current formatLogParam function mutates string payloads
by using .replace(": ", ":"), which will change legitimate top-level string
values; remove that replacement and only trim each line from prettyPrintJson
(i.e., keep prettyPrintJson(param).split("\n").map(line =>
line.trim()).join("").trim()) so you preserve original string contents; update
the formatLogParam implementation (referenced by function name formatLogParam
and the use of prettyPrintJson) accordingly.
🪄 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: f1e73f32-f53d-4b74-a87b-b90d1fda1800
📒 Files selected for processing (21)
.changeset/cuddly-steaks-jog.md.changeset/warm-geese-fall.mdapps/ensindexer/ponder/ponder.config.tsapps/ensindexer/ponder/src/api/handlers/ensnode-api.tsapps/ensindexer/ponder/src/api/index.tsapps/ensindexer/src/lib/__test__/mockLogger.tsapps/ensindexer/src/lib/dns-helpers.test.tsapps/ensindexer/src/lib/dns-helpers.tsapps/ensindexer/src/lib/ensdb-writer-worker/ensdb-writer-worker.test.tsapps/ensindexer/src/lib/ensdb-writer-worker/ensdb-writer-worker.tsapps/ensindexer/src/lib/ensdb-writer-worker/singleton.tsapps/ensindexer/src/lib/ensdb/migrate-ensnode-schema.tsapps/ensindexer/src/lib/ensraibow-api-client.tsapps/ensindexer/src/lib/graphnode-helpers.test.tsapps/ensindexer/src/lib/graphnode-helpers.tsapps/ensindexer/src/lib/logger.tsapps/ensindexer/src/lib/version-info.tsapps/ensindexer/src/plugins/ensv2/handlers/ensv1/NameWrapper.tspackages/ponder-sdk/src/deserialize/ponder-app-context.tspackages/ponder-sdk/src/local-ponder-client.mock.tspackages/ponder-sdk/src/ponder-app-context.ts
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
…mlined logger implementation across ENSIndexer app.
tk-o
force-pushed
the
feat/ensindexer-logger-from-ponder
branch
from
08ff1ef to
b8161e7
Compare
Contributor
Author
|
@greptile review |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/ensindexer/src/lib/version-info.ts (1)
67-70:⚠️ Potential issue | 🟡 MinorUse
logger.warnfor this handled fallback path.This branch recovers by returning
"unknown", so logging at error level will over-report non-fatal behavior.Proposed fix
- logger.error({ + logger.warn({ msg: `Could not find version for ${packageName}`, error: buildLogError(error), });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ensindexer/src/lib/version-info.ts` around lines 67 - 70, The catch branch in getVersionForPackage (the block that currently calls logger.error with msg `Could not find version for ${packageName}` and error: buildLogError(error)) is a handled fallback that returns "unknown", so change the logging level from logger.error to logger.warn (and keep the same message and error payload) to avoid over-reporting non-fatal conditions; update the call that references packageName and buildLogError(error) accordingly.
🤖 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/__test__/mockLogger.ts`:
- Around line 4-9: The test logger stub mockLogger is missing the trace method
that exists on the runtime PonderAppLogger, causing tests that call
logger.trace(...) to fail; update the mock by adding a trace: vi.fn() property
to the mockLogger object and any other test logger stubs (the other instance
noted) so the test stub matches the PonderAppLogger contract.
In `@apps/ensindexer/src/lib/ensdb-writer-worker/singleton.ts`:
- Around line 39-42: The logger.error call in singleton.ts is missing the module
field used elsewhere; update the call to include module:
formatLogParam("EnsDbWriterWorker") alongside the existing msg and error fields
and ensure formatLogParam is imported together with buildLogError (i.e., add
formatLogParam to the import list) so logs match the structured format used by
other EnsDbWriterWorker entries.
---
Duplicate comments:
In `@apps/ensindexer/src/lib/version-info.ts`:
- Around line 67-70: The catch branch in getVersionForPackage (the block that
currently calls logger.error with msg `Could not find version for
${packageName}` and error: buildLogError(error)) is a handled fallback that
returns "unknown", so change the logging level from logger.error to logger.warn
(and keep the same message and error payload) to avoid over-reporting non-fatal
conditions; update the call that references packageName and buildLogError(error)
accordingly.
🪄 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: f9103bb3-bb1a-41ec-9edf-78e44cc74e65
📒 Files selected for processing (12)
.changeset/fast-coins-grab.mdapps/ensindexer/ponder/src/api/index.tsapps/ensindexer/src/lib/__test__/mockLogger.tsapps/ensindexer/src/lib/dns-helpers.test.tsapps/ensindexer/src/lib/dns-helpers.tsapps/ensindexer/src/lib/ensdb-writer-worker/ensdb-writer-worker.test.tsapps/ensindexer/src/lib/ensdb-writer-worker/ensdb-writer-worker.tsapps/ensindexer/src/lib/ensdb-writer-worker/singleton.tsapps/ensindexer/src/lib/graphnode-helpers.test.tsapps/ensindexer/src/lib/logger.tsapps/ensindexer/src/lib/version-info.tspackages/ponder-sdk/src/ponder-app-context.ts
tk-o
commented
Apr 2, 2026
Comment on lines
+63
to
+72
| logger.warn({ | ||
| msg: `ENSRainbow health check failed`, | ||
| attempt: attemptNumber, | ||
| retriesLeft, | ||
| error: buildLogError(retriesLeft === 0 ? error : undefined), | ||
| ensRainbowInstance: formatLogParam(ensRainbowUrl.href), | ||
| advice: formatLogParam( | ||
| `This might be due to ENSRainbow having a cold start, which can take 30+ minutes.`, | ||
| ), | ||
| }); |
Contributor
Author
There was a problem hiding this comment.
This produces a really nice log message:
17:38:43.661 WARN ENSRainbow health check failed attempt=1 retriesLeft=60 ensRainbowInstance="http://localhost:3223/" advice="This might be due to ENSRainbow having a cold start, which can take 30+ minutes."
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/ensindexer/src/lib/version-info.ts (1)
67-70:⚠️ Potential issue | 🟡 MinorUse
logger.warninstead oflogger.errorfor gracefully handled exceptions.This exception is caught and handled gracefully by returning
"unknown". Since the application continues to function normally,logger.warnis the appropriate level. Reservelogger.errorfor unrecoverable failures or critical issues that require immediate attention.🛠️ Proposed fix
- logger.error({ + logger.warn({ msg: `Could not find version for ${packageName}`, error, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ensindexer/src/lib/version-info.ts` around lines 67 - 70, The caught exception in version-info.ts that logs the missing package version is currently using logger.error even though the code handles it gracefully (returns "unknown"); change the call to logger.warn so the event is recorded at warning level instead of error, keeping the same message and error object. Locate the logger.error call that references packageName and error in the getVersion / version retrieval logic and replace it with logger.warn to reflect that this is non-fatal.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ponder-sdk/src/deserialize/ponder-app-context.ts`:
- Around line 46-55: The deserialize pipeline is double-wrapping the runtime
logger: schemaPonderAppLogger applies .transform(wrapPonderAppLogger) and the
same transform is applied again later when the raw schema is piped into the
final schema; remove the redundant second wrap so the runtime logger is wrapped
exactly once. Locate schemaPonderAppLogger and any other schemas listed (lines
referenced around 64, 78, 94) that call wrapPonderAppLogger and eliminate the
duplicate .transform(wrapPonderAppLogger) application (keep the transform in
only one canonical place), or conditionally apply the wrapper only when the
input is not already wrapped to prevent double-wrapping.
---
Duplicate comments:
In `@apps/ensindexer/src/lib/version-info.ts`:
- Around line 67-70: The caught exception in version-info.ts that logs the
missing package version is currently using logger.error even though the code
handles it gracefully (returns "unknown"); change the call to logger.warn so the
event is recorded at warning level instead of error, keeping the same message
and error object. Locate the logger.error call that references packageName and
error in the getVersion / version retrieval logic and replace it with
logger.warn to reflect that this is non-fatal.
🪄 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: ab6c8cf0-315e-4eb7-a6ec-2f330d70ebe8
📒 Files selected for processing (18)
apps/ensindexer/ponder/ponder.config.tsapps/ensindexer/ponder/src/api/handlers/ensnode-api.tsapps/ensindexer/ponder/src/api/index.tsapps/ensindexer/src/lib/__test__/mockLogger.tsapps/ensindexer/src/lib/dns-helpers.tsapps/ensindexer/src/lib/ensdb-writer-worker/ensdb-writer-worker.tsapps/ensindexer/src/lib/ensdb-writer-worker/singleton.tsapps/ensindexer/src/lib/ensdb/migrate-ensnode-schema.tsapps/ensindexer/src/lib/ensrainbow/singleton.tsapps/ensindexer/src/lib/local-ponder-client.tsapps/ensindexer/src/lib/local-ponder-context.tsapps/ensindexer/src/lib/logger.tsapps/ensindexer/src/lib/version-info.tspackages/ponder-sdk/src/deserialize/ponder-app-context.tspackages/ponder-sdk/src/deserialize/ponder-app-logger.tspackages/ponder-sdk/src/index.tspackages/ponder-sdk/src/ponder-app-context.tspackages/ponder-sdk/src/ponder-app-logger.ts
Contributor
Author
|
@greptile review |
Contributor
Author
|
@shrugs nice suggestions! I've put the logger wrapper functionality into Ponder SDK so anyone could use the logger with nice auto-formatting applied 👍 It's in |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ensnode-sdk/src/shared/config/pretty-printing.ts`:
- Around line 33-34: The stringifyConfig function passes the indentation arg to
JSON.stringify inverted; currently options.pretty true yields compact output.
Update the call in stringifyConfig to pass options.pretty ? 2 : undefined (i.e.,
use 2 spaces when pretty is true, undefined when false) while keeping the
configJSONReplacer argument intact so pretty=true produces an indented string
and pretty=false produces compact output.
- Line 31: Remove the redundant `@returns` JSDoc tag in the JSDoc block in
pretty-printing.ts (the doc block describing the function that returns "The JSON
string representation of the config object"); edit the JSDoc immediately above
that function to delete the lone "@returns The JSON string representation of the
config object" line so the summary remains but the redundant `@returns` tag is
removed.
🪄 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: c7b2c7c0-9eaf-4ef0-8ea4-49f1c14be538
📒 Files selected for processing (3)
apps/ensrainbow/src/commands/server-command.tspackages/ensnode-sdk/src/shared/config/pretty-printing.tspackages/ponder-sdk/src/deserialize/ponder-app-context.ts
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 28 out of 28 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+48
to
+57
| function formatLogValue(value: unknown): unknown { | ||
| // Primitives pass through | ||
| if (isPrimitive(value)) return value; | ||
|
|
||
| // Error instances pass through (handled specially by logger) | ||
| if (value instanceof Error) return value; | ||
|
|
||
| // Otherwise JSON stringify with replacer | ||
| return JSON.stringify(value, replacer); | ||
| } |
There was a problem hiding this comment.
formatLogValue uses JSON.stringify for non-primitive objects, which will throw for common values in the EVM/TypeScript ecosystem (notably objects containing nested bigint values) and for circular structures. A logger wrapper should avoid throwing, otherwise a logging attempt can crash the app.
Consider extending the replacer to safely serialize bigint (e.g. to string) and wrapping JSON.stringify in a try/catch with a safe fallback (like String(value) or a sentinel like "[Unserializable]").
Part of streamling formatting for logging config objects
tk-o
force-pushed
the
feat/ensindexer-logger-from-ponder
branch
from
d80ea92 to
90c8aec
Compare
Contributor
Author
|
@greptile review |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 28 out of 28 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
PonderAppContextdata model.PonderAppContextdata model into ENSIndexer app.@/lib/loggermodule which includes theloggerexport.formatLogParamhelper to format additional params passed along the log message.console.logwithlogger.info(orlogger.debugif deemed more suitable).console.warnwithlogger.warn.console.errorwithlogger.error.Log output after the changes
Why
ENSIndexer logs output has been inconsitent, as pointed out in this PR comment with feedack.
Inconsitent Log output
Different approach to formatting different log messages.
Testing
Notes for Reviewer (Optional)
formatLogParamallows to manage white-spaces for printed logs params. Ideally, single log message should be printend within a single line (no pretty-priting of JSON objects). This is to improve logs readability on the cloud platform side.Pre-Review Checklist (Blocking)