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

New Issue

2026-05-05T22:06:08Z

mahseiah_ai commented

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

Problem

Logger.getEffectiveThreshold() currently uses a truthiness check:

if (this.threshold) {
  return this.threshold
}

Since LogLevel.ALL = 0, an explicit threshold of LogLevel.ALL is treated as “unset”. That means a child logger cannot override a stricter parent threshold back down to ALL.

Reproduction

const parent = new Logger("parent", undefined, LogLevel.WARN)
const child = new Logger("child", parent, LogLevel.ALL)

const appender = {
  threshold: LogLevel.ALL,
  messages: [] as LogMessage[],
  appendMessage(msg: LogMessage) {
    this.messages.push(msg)
  },
}

child.appenders = [appender]
child.info("should be logged")

Expected

child.info(...) should be logged, because the child explicitly set its threshold to LogLevel.ALL.

Actual

The child falls through to the parent threshold (WARN), so INFO is dropped.

Scope

Change Logger.getEffectiveThreshold() to use an explicit undefined/null check instead of truthiness
Add a regression test where a child logger sets LogLevel.ALL under a stricter parent threshold
Review related threshold logic for the same 0-is-falsy pattern

Notes

This is a real functional bug.
The current logger test added in PR #3 does not catch this case because it constructs a logger without a stricter parent, so the fallback path still returns LogLevel.ALL.
Related follow-up: issue #4 tracks similar truthiness cleanup in the appenders, where the current behavior is correct but fragile.

## Problem `Logger.getEffectiveThreshold()` currently uses a truthiness check: ```ts if (this.threshold) { return this.threshold } ``` Since `LogLevel.ALL = 0`, an explicit threshold of `LogLevel.ALL` is treated as “unset”. That means a child logger cannot override a stricter parent threshold back down to `ALL`. ## Reproduction ```ts const parent = new Logger("parent", undefined, LogLevel.WARN) const child = new Logger("child", parent, LogLevel.ALL) const appender = { threshold: LogLevel.ALL, messages: [] as LogMessage[], appendMessage(msg: LogMessage) { this.messages.push(msg) }, } child.appenders = [appender] child.info("should be logged") ``` ### Expected `child.info(...)` should be logged, because the child explicitly set its threshold to `LogLevel.ALL`. ### Actual The child falls through to the parent threshold (`WARN`), so `INFO` is dropped. ## Scope - [ ] Change `Logger.getEffectiveThreshold()` to use an explicit `undefined`/`null` check instead of truthiness - [ ] Add a regression test where a child logger sets `LogLevel.ALL` under a stricter parent threshold - [ ] Review related threshold logic for the same `0`-is-falsy pattern ## Notes - This is a real functional bug. - The current logger test added in PR #3 does **not** catch this case because it constructs a logger without a stricter parent, so the fallback path still returns `LogLevel.ALL`. - Related follow-up: issue #4 tracks similar truthiness cleanup in the appenders, where the current behavior is correct but fragile.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: jdb/js-logging#5