HarperFast / HarperFast/harper
External logger silently loses rotation: an `external` block without `rotation` overwrites the inherited value and tears down the rotator (unbounded log growth)
- Dominant language
- JavaScript
- Stars
- 89
- Forks
- 10
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 200
Description
An `external` logging block that does not specify `rotation` **silently disables log rotation** for that logger, so `maxSize` is never enforced and the file grows without bound. Reported in the field at ~200 GB.
## Root cause
`utility/logging/harper_logger.ts` `updateLogger()` assigns rotation unconditionally, with no inheritance from the main logger:
```ts
function updateLogger(logger: any, logOptions: any, name?: string) {
logger.rotation = logOptions.rotation; // :104 — no fallback
let path = logOptions.path;
if (path) { ... }
else if (logOptions.root) { path = join(logOptions.root, logName); }
else { path = mainLogger.path; ... } // :105-111 — path DOES inherit
```
Compare the two: `path` has a full fallback chain ending at `mainLogger.path`, and `level`/`tag` are handled similarly. `rotation` has none — so `logOptions.rotation === undefined` overwrites whatever was inherited.
`getFileLogger()` then does not just skip building a rotator, it **tears down any existing one**:
```ts
if (isMainThread && JSON.stringify(rotation) !== JSON.stringify(logger.rotation)) {
logger.rotation = rotation;
setTimeout(() => {
logger.rotator?.end();
if (!rotation) return; // :751-752
const { logRotator } = require('./logRotator');
logger.rotator = logRotator({ logger, ...rotation });
}, 100);
}
```
So the outcome for an `external` block with no `rotation` key is: rotation inherited → overwritten with `undefined` → existing rotator ended → no rotator rebuilt → unbounded growth. `maxSize` in the main `logging` block appears to be configured and is silently not in effect for that logger.
## Why it is easy to miss
- The config *looks* correct: `maxSize` is set at the top level, and every other inherited field (`path`, `level`, `tag`) does inherit, so there is no reason to expect `rotation` not to.
- Nothing warns. There is no log line saying rotation was disabled for this logger — the only symptom is disk consumption, which shows up long after the deploy that caused it.
- An instance that restarts regularly re-runs the main-logger path and can look healthy; long-uptime instances accumulate.
## Suggested direction
Give `rotation` the same inheritance as `path`/`level`/`tag` — fall back to the main logger's rotation when a child block omits it:
```ts
logger.rotation = logOptions.rotation ?? mainLogger.rotation;
```
Worth deciding alongside it whether an explicit opt-out is needed (`rotation: false`) so "inherit" and "deliberately unrotated" are distinguishable — with the current shape they are the same value. A warn on the transition from a rotating to a non-rotating logger would also have surfaced this at deploy time rather than at 200 GB.
## Verification
Source-confirmed on `main` (`eb702ee52`); the code path above is unchanged from the original report. Not reproduced end-to-end here — a disk-growth repro needs a long run or a small `maxSize` plus sustained volume, and the source chain is unambiguous.
Found by exploratory QA; adjudicated and re-verified during a queue triage pass.
Contributor guide
Research direction
Start in utility/logging/harper_logger.ts at updateLogger() and getFileLogger(), tracing how rotation is assigned and how an existing rotator is ended. Verify that an external block omitting rotation preserves the main logger’s rotation and that the rotator remains active; also determine whether an explicit opt-out needs separate handling.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- nodejs, typescript
- Domain
- backend, observability-sre
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100