Skip to content

feat: replace console.debug with injectable logger interface#542

Draft
reneaaron wants to merge 3 commits intomasterfrom
feat/logger-interface
Draft

feat: replace console.debug with injectable logger interface#542
reneaaron wants to merge 3 commits intomasterfrom
feat/logger-interface

Conversation

@reneaaron
Copy link
Copy Markdown
Member

@reneaaron reneaaron commented Apr 14, 2026

Summary

  • Introduce a Logger interface (src/logger.ts) with a noop default, exported from the package root
  • Remove "debug" from the ESLint no-console allow list so console.debug is now a lint error
  • Add optional logger to NWCClient, NWAClient, NWCWalletService, and OauthWeblnProvider options — defaults to silent noop, consumers can pass { logger: console } to opt in
  • Clean up all commented-out console.debug lines

Test plan

  • ESLint passes with no errors
  • TypeScript compiles with no errors
  • Pre-commit hooks (lint-staged, commitlint) pass
  • Verify existing tests still pass (yarn test)
  • Smoke-test with { logger: console } to confirm debug output appears

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Introduced a Logger interface for custom logging configuration. Logger can now be optionally injected into NWA, NWC, and OAuth clients for flexible debug output control.
  • Chores

    • Updated ESLint configuration to enforce stricter console usage policies.
    • Removed commented-out debug statements throughout the codebase.

reneaaron and others added 2 commits April 7, 2026 23:27
console.info writes to stdout, which pollutes output for agents
and tools consuming the SDK. console.debug writes to stderr instead.
Also update eslint no-console rule to disallow console.info going forward.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove console.debug from the ESLint allow list and introduce a Logger
interface with a noop default. All debug output now routes through
this.logger.debug(), keeping stdout clean by default while letting
consumers opt in via { logger: console } or a custom logger.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 14, 2026

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 12a3347c-93b1-4d12-8b85-bc3e7c8fa0e5

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/logger-interface

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/nwc/NWAClient.ts (1)

31-55: ⚠️ Potential issue | 🟡 Minor

Propagate injected logger to internally created NWCClient.

NWAClient accepts logger injection (Lines 31, 55), but the new NWCClient(...) created in the subscribe flow does not receive it (Line 198), so downstream client debug logs remain disabled.

Suggested patch
                 const client = new NWCClient({
                   relayUrls: this.options.relayUrls,
                   secret: this.appSecretKey,
                   walletPubkey: event.pubkey,
+                  logger: this.logger,
                 });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/nwc/NWAClient.ts` around lines 31 - 55, The NWAClient accepts an injected
logger (this.logger / options.logger) but does not pass it into the internally
created NWCClient in the subscribe flow (where new NWCClient(...) is
constructed), so downstream logs stay disabled; update the code that constructs
NWCClient to pass this.logger (or options.logger || noopLogger) into NWCClient's
constructor or options so the inner client receives the same logger, ensuring
you reference the NWAClient constructor/this.logger and the place where new
NWCClient(...) is created (subscribe flow) when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/nwc/NWAClient.ts`:
- Around line 31-55: The NWAClient accepts an injected logger (this.logger /
options.logger) but does not pass it into the internally created NWCClient in
the subscribe flow (where new NWCClient(...) is constructed), so downstream logs
stay disabled; update the code that constructs NWCClient to pass this.logger (or
options.logger || noopLogger) into NWCClient's constructor or options so the
inner client receives the same logger, ensuring you reference the NWAClient
constructor/this.logger and the place where new NWCClient(...) is created
(subscribe flow) when making the change.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: dce1ebad-63c4-4284-aaec-8a14a3cb1c03

📥 Commits

Reviewing files that changed from the base of the PR and between 08a6cd7 and a4dbfd9.

📒 Files selected for processing (7)
  • .eslintrc.json
  • src/index.ts
  • src/logger.ts
  • src/nwc/NWAClient.ts
  • src/nwc/NWCClient.ts
  • src/nwc/NWCWalletService.ts
  • src/webln/OauthWeblnProvider.ts

The NWCClient created inside the subscribe flow was not receiving the
injected logger, so downstream debug logs stayed silenced even when a
logger was provided. Pass this.logger through.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant