Skip to content

fix: replace console.warn with structured logger in SDK router#609

Merged
realfishsam merged 1 commit into
mainfrom
fix/304-396-router-console
May 24, 2026
Merged

fix: replace console.warn with structured logger in SDK router#609
realfishsam merged 1 commit into
mainfrom
fix/304-396-router-console

Conversation

@realfishsam

Copy link
Copy Markdown
Contributor

Fixes #396

@realfishsam

Copy link
Copy Markdown
Contributor Author

PR Review: PASS (NOT VERIFIED)

What This Does

Adds a new structured logger module (sdks/typescript/pmxt/logger.ts) to the TypeScript SDK and replaces a single console.warn call in router.ts with logger.warn.

Blast Radius

  • New file: sdks/typescript/pmxt/logger.ts (78 lines). Introduces a logger singleton with debug/info/warn/error methods, a [pmxt] prefix, and level gating via PMXT_LOG_LEVEL env var.
  • Modified file: sdks/typescript/pmxt/router.ts -- one line changed in the deprecated fetchMatches method.

Findings

  • Module-level mutable state: let currentLevel: LogLevel = getDefaultLevel() is module-scoped mutable state. logger.setLevel() mutates it. This means the log level is global across all Exchange/Router instances in the same process. This is standard for loggers but worth noting.
  • getDefaultLevel() reads process.env at module load time. If PMXT_LOG_LEVEL is set after import, the initial level won't reflect it. Users can call logger.setLevel() to override. This is acceptable.
  • No validation on PMXT_LOG_LEVEL. If the env var is set to an invalid value (e.g., PMXT_LOG_LEVEL=verbose), it will be cast to LogLevel without validation. shouldLog() will then look up LEVEL_PRIORITY[currentLevel] which returns undefined, and undefined >= LEVEL_PRIORITY[level] is false -- so all logging would be silently suppressed. Consider adding validation in getDefaultLevel().
  • typeof process !== 'undefined' guard enables browser compatibility. Good.
  • Only one call site updated. The console.warn in router.ts is the only usage. If the goal is to replace all console.* calls in the SDK, this is incomplete but could be intentional as a first step.
  • Not verified because the new logger module introduces a new public export (logger) that SDK consumers could import. Need to confirm this aligns with the SDK's public API surface and doesn't interfere with the generated client code.

Semver Impact

minor -- new logger export added to the SDK package, no breaking changes.

Risk

Low-Medium. The missing env var validation is the main concern -- an invalid PMXT_LOG_LEVEL silently kills all logging.

@realfishsam realfishsam merged commit 4f88f60 into main May 24, 2026
8 of 11 checks passed
@realfishsam realfishsam deleted the fix/304-396-router-console branch May 24, 2026 17:03
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.

console: sdks/typescript/pmxt/router.ts:207 — console.warn for deprecated fetchMatches in SDK

1 participant