Fix LogLevel.ALL threshold truthiness checks #4

New Issue

2026-05-05T20:12:57Z

mahseiah_ai commented

2026-05-05 20:12:57 +00:00

Problem

Three appender implementations use a truthiness guard for threshold checks:

if (this.threshold && msg.level < this.threshold) return;

Since LogLevel.ALL = 0 is falsy in JavaScript, the expression 0 && ... short-circuits to 0 (falsy), meaning the if block is never entered and messages pass through. This is correct behavior by accident — it works today but is fragile and misleading.

The fix is to use an explicit !== undefined check instead:

if (this.threshold !== undefined && msg.level < this.threshold) return;

Scope

src/console-log-appender.ts — replace truthiness check
src/api-log-appender.ts — replace truthiness check
src/buffer-log-appender.ts — replace truthiness check
Add/confirm regression test coverage for explicit LogLevel.ALL = 0 edge case (tests exist but may need tightening to actually fail on the buggy code)

Notes

This is a cleanup follow-up, not a blocker. CI is green and current behavior is correct by coincidence.
The existing tests in PR #3 already cover the ALL=0 scenario (see test/console-log-appender.test.ts "threshold of ALL (0) does not cause falsy guard to block messages") but the test comment literally acknowledges "correct behavior by accident."
Fixing the guard makes the intent explicit and prevents future bugs if the logic around the guard changes.

## Problem Three appender implementations use a truthiness guard for threshold checks: ``` if (this.threshold && msg.level < this.threshold) return; ``` Since `LogLevel.ALL = 0` is falsy in JavaScript, the expression `0 && ...` short-circuits to `0` (falsy), meaning the `if` block is never entered and messages pass through. This is **correct behavior by accident** — it works today but is fragile and misleading. The fix is to use an explicit `!== undefined` check instead: ``` if (this.threshold !== undefined && msg.level < this.threshold) return; ``` ## Scope - [ ] `src/console-log-appender.ts` — replace truthiness check - [ ] `src/api-log-appender.ts` — replace truthiness check - [ ] `src/buffer-log-appender.ts` — replace truthiness check - [ ] Add/confirm regression test coverage for explicit `LogLevel.ALL = 0` edge case (tests exist but may need tightening to actually fail on the buggy code) ## Notes - This is a cleanup follow-up, not a blocker. CI is green and current behavior is correct by coincidence. - The existing tests in PR #3 already cover the ALL=0 scenario (see `test/console-log-appender.test.ts` "threshold of ALL (0) does not cause falsy guard to block messages") but the test comment literally acknowledges "correct behavior by accident." - Fixing the guard makes the intent explicit and prevents future bugs if the logic around the guard changes.

mahseiah_ai referenced this issue

2026-05-05 22:06:08 +00:00

Fix Logger.getEffectiveThreshold() to honor explicit LogLevel.ALL #5

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: jdb/js-logging#4