Improve logging in javascript driver

[krishna-yb]

Replace [object Object] with proper JSON.stringify for complete silly logs

[krishna-yb]

Hey @ashetkar @HarshDaryani896
Please take a look

[HarshDaryani896]

Overview

Replaces string-concatenated log statements that rendered Maps/objects as [object Object] (and host,[object Object] for Maps) with JSON.stringify(Array.from(map.entries())) / JSON.stringify(config). The intent is sound and the happy-path output is correct — these logs become genuinely readable, which matters since the driver leans heavily on silly/debug logs for load-balancing diagnostics.

🔴 Blocking: password (and other secrets) leak in the constructor log

logger.silly("Received connection string: " + (typeof config === 'string' ? config : JSON.stringify(config, null, 2)))

• When config is an object it typically contains password (e.g. { user, password, host, ... }), so this now serializes the plaintext password into the log.
• It contradicts the intent stated a few lines below in the same constructor:

  // "hiding" the password so it doesn't show up in stack traces or if the client is console.logged

• It's worse than a console line: the winston logger has a File transport (app.log), so the password gets persisted to disk whenever LOG_LEVEL=silly — exactly what the test/example setups enable.
• The string-form branch leaks too: a connection string like postgresql://user:pass@host/db embeds the password. (The old [object Object] was useless but safe.)

Suggested fix — redact before logging:

const safe = typeof config === 'string'
  ? config.replace(/:\/\/([^:]+):[^@]+@/, '://$1:****@')
  : JSON.stringify({ ...config, password: config?.password ? '****' : undefined }, null, 2)
logger.silly("Received connection string: " + safe)

🟠 Log arguments are evaluated eagerly — throw risk + wasted work

logger.silly(msg) computes msg before the call, regardless of the active level (default info, so silly/debug are suppressed but the string is still built).

• Throw risk: JSON.stringify throws on circular references / BigInt. The constructor and getLeastLoadedServer now run it unconditionally — a config with a circular ref would make new Client() throw from a log line. The previous "" + config / [...map] coercion never throws.
• Performance: getLeastLoadedServer() is on the per-connect load-balancing hot path, and the constructor runs for every client/pool checkout. JSON.stringify(...) (plus null, 2 pretty-printing) now runs on each even when the log is discarded. Ideally guard with a level check so the cost is only paid when the level is active.

Both pre-existed as eager concatenations, but this PR makes each site materially more expensive and adds the throw surface.

🟡 Minor: inconsistent Map serialization

Most sites use .entries(), but failedHosts uses the bare form in two places:

JSON.stringify(Array.from(Client.failedHosts))                 // iterateHostList, updateConnectionMapAfterRefresh
JSON.stringify(Array.from(Client.hostServerInfoRR.entries()))  // everywhere else

Array.from(map) and Array.from(map.entries()) are equivalent, so this is cosmetic — worth aligning. A small shared helper (mapToJSON(m) / safeStringify) would DRY up all ~10 call sites and is the natural place to add the throw-safety and redaction.

Recommendation

Nice readability improvement and I'd like to see it land, but the password leak should be fixed before merge, and the eager-JSON.stringify throw risk is worth addressing. A single throw-safe safeStringify/mapToJSON helper resolves the leak, the throw risk, and the style nit at once.

---

automated · Claude Code (Opus 4.8)

[HarshDaryani896]

Thanks for the thorough revision — this went well beyond the minimum and centralized everything into a clean logging utility layer. All three points from the earlier review are addressed, and I verified the new logger.js helpers against a battery of inputs (not just by reading):

• ✅ Secret redaction — object password, token, TLS key/pfx/passphrase (only when the object looks like a TLS config), and passwords embedded in connectionString (both top-level and nested) are all masked to ****. Confirmed the plaintext no longer reaches the serialized output.
• ✅ Throw-safe — circular refs render as [Circular], BigInt as a string, and any other error is caught, so a log line can no longer break the code path it instruments.
• ✅ Lazy/level-gated — logLazy does not invoke the builder at all when the level is disabled (verified a throwing builder is harmless when disabled and runs once when enabled), so the hot paths (getLeastLoadedServer, constructor) pay nothing when silly/debug are off.
• ✅ Consistency — all call sites now go through collectionToJSON.

Also confirmed the installed winston (3.14.2) has logger.isLevelEnabled, so logLazy in the constructor won't throw.

Two optional, non-blocking nits:

1. Shared (non-circular) references get a false [Circular]. safeStringify({a: shared, b: shared}) marks b as [Circular] even though it's just a repeated reference, not a cycle — the seen set tracks visited objects without tracking the current path. Harmless for logs (you only lose a duplicate's expansion), but a one-line comment would save a future reader from chasing a phantom cycle.
2. Over-redaction of benign keys whose name contains a secret substring, e.g. tokenBucket -> ****. This is fail-safe and fine for logging; noting it only so it's a known trade-off.

Trivial: logger.js is missing a trailing newline.

LGTM 👍

---

automated · Claude Code (Opus 4.8)