Skip to content

fix: use configured SDK logger - #1

Open
DYSfu wants to merge 1 commit into
BitgesellOfficial:mainfrom
DYSfu:fix-configured-sdk-logger
Open

fix: use configured SDK logger#1
DYSfu wants to merge 1 commit into
BitgesellOfficial:mainfrom
DYSfu:fix-configured-sdk-logger

Conversation

@DYSfu

@DYSfu DYSfu commented May 21, 2026

Copy link
Copy Markdown

Summary:

  • Wire SDKConfig.logger into the Address, Blockchain, Mempool, and Transaction wrappers.
  • Add request-failure coverage for the configured logger path.
  • Remove the stray describe.only so the full Jest suite runs.

Validation:

  • npm ci --legacy-peer-deps
  • npx jest src/__tests__/logger.test.ts --runInBand -> 4 tests passed
  • npm run build:main
  • npm test -- --runInBand -> 26 tests passed
  • git diff --check

Related:

@MyTH-zyxeon

Copy link
Copy Markdown

Review assist for maintainers:

This looks like a useful SDK correctness/testability fix: the Address, Blockchain, Mempool, and Transactions wrappers now use SDKConfig.logger instead of falling back to a no-op instance field, and the new Jest coverage mocks axios failures across all four wrappers. Removing describe.only from address.test.ts also restores full-suite visibility.

Suggested merge checks:

  • Run npm ci --legacy-peer-deps, the focused logger Jest test, npm run build:main, and the full Jest suite from the PR body.
  • Confirm the SDKConfig.logger type matches the value passed by the wrappers on failures. If the catch path passes an Error object, either type the logger accordingly or normalize to a string/message before calling it.
  • Verify the logger tests remain fully mocked and do not hit the public Bitaps API or require live RPC credentials.
  • Check overlap with the separate full-suite/describe.only cleanup PRs so maintainers merge in an order that avoids duplicate or stale diffs.

I do not see wallet/key, signing, broadcast, live RPC, or live-chain behavior changes in this patch as written; the acceptance criterion is that configured logger callbacks are actually invoked on request failures and the whole SDK test suite is no longer accidentally focused.

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.

2 participants