Summary
[factory] error {} — the most alarming line factory prints is also the least informative. A plain Error logged as the metadata object serializes to {} because Error.message and Error.stack are non-enumerable, so JSON.stringify drops them. The error is counted in the errors metric but is undiagnosable from the log.
Observed
A real dispatch (hoopsheet#30, factory 0.1.17), verbatim:
[factory-sdk] waiting for spawned agents to exit before shutting down the owned relay broker {"agents":["ar-30-impl-hoopsheet","ar-30-review"],"timeoutMs":1800000,"remainingCount":2}
[factory] error {}
[factory] error {}
Two errors occurred. The log says nothing about either — no message, no stack, no type, no issue.
The same line is sometimes useful, which makes it worse:
[factory] error {"code":"http_500","retryable":true,"status":500,"data":{"error":"internal reply dropped","success":false},"name":"HarnessDriverProtocolError"}
HarnessDriverProtocolError carries enumerable own properties, so it prints. A bare Error prints {}. Identical log line, wildly different information, depending on the error class — so you cannot tell from the log whether "nothing was printed" means "nothing to say" or "everything was dropped."
Root cause
src/orchestrator/factory.ts:4087:
#error(error: unknown, issue?: IssueRef): void {
this.#increment('errors')
this.#logger.error?.('[factory] error', error) // <-- raw Error -> {}
this.#emit('error', { error, ...describeError(error), issue }) // <-- describeError() right here
}
describeError already exists in the same file (factory.ts:6547) and already extracts exactly what's missing:
const describeError = (error: unknown): { errorMessage: string; errorStack?: string } => {
if (error instanceof Error) { ... }
Line 4090 uses it. Line 4089 doesn't. The emitted event is diagnosable; the log line — the thing a human actually reads — is not.
Fix
#error(error: unknown, issue?: IssueRef): void {
this.#increment('errors')
const details = describeError(error)
this.#logger.error?.('[factory] error', { ...details, issue: issue?.key, ...errorFields(error) })
this.#emit('error', { error, ...details, issue })
}
Include at minimum errorMessage, errorStack, the error name/code when present, and the issue key — [factory] error {} during a dispatch doesn't even say which issue it belongs to.
This is systemic, not one line
27 call sites in factory.ts alone pass a raw error as the log metadata (grep -cE "logger\.(error|warn|info)\?\.\([^,]+, *(error|err)\)"). Every one has the same defect. Examples:
factory.ts:634 this.#logger.warn?.('[factory] failed to refresh stopping heartbeat', error)
factory.ts:663 this.#logger.warn?.('[factory] failed to refresh live daemon heartbeat', error)
factory.ts:707 this.#logger.warn?.('[factory] live subscription high-watermark unavailable', error)
factory.ts:738 this.#logger.warn?.('[factory] live subscription poll failed', error)
factory.ts:762 this.#logger.warn?.('[factory] live issue event drain failed', error)
factory.ts:989 this.#logger.warn?.('[factory] PR completion sweep failed', error)
Suggested approach:
- Fix
#error() first — it's the loudest and it increments a metric.
- Normalise error serialization in the logger so a raw
Error can never render as {} regardless of call site. That fixes all 27 without touching 27 lines, and prevents the 28th.
- Optionally lint against passing a bare
Error/unknown as the metadata object.
Why it matters beyond tidiness
- These lines are counted errors (
#increment('errors')) — a metric you can't act on.
[factory] error {} reads as something is broken to anyone running a dispatch, with zero way to tell whether it's benign teardown noise or a real failure. In practice it appears on every run (I've seen it on 4/4 clean dispatches that produced correct PRs), which trains operators to ignore an error-level log — the worst possible outcome for the one line that should mean "look here."
- It hides real bugs. The
internal reply dropped / failed to release ar-NN-review teardown failure is currently only diagnosable because that particular error class happens to serialize. A plain Error in the same path would be invisible.
Related: #67/#69 (reviewer-hang teardown — the errors that surface here), #82 (agent-name collision — diagnosed only because resume-already-exists was logged with real fields).
Summary
[factory] error {}— the most alarming line factory prints is also the least informative. A plainErrorlogged as the metadata object serializes to{}becauseError.messageandError.stackare non-enumerable, soJSON.stringifydrops them. The error is counted in theerrorsmetric but is undiagnosable from the log.Observed
A real dispatch (
hoopsheet#30, factory 0.1.17), verbatim:Two errors occurred. The log says nothing about either — no message, no stack, no type, no issue.
The same line is sometimes useful, which makes it worse:
HarnessDriverProtocolErrorcarries enumerable own properties, so it prints. A bareErrorprints{}. Identical log line, wildly different information, depending on the error class — so you cannot tell from the log whether "nothing was printed" means "nothing to say" or "everything was dropped."Root cause
src/orchestrator/factory.ts:4087:describeErroralready exists in the same file (factory.ts:6547) and already extracts exactly what's missing:Line 4090 uses it. Line 4089 doesn't. The emitted event is diagnosable; the log line — the thing a human actually reads — is not.
Fix
Include at minimum
errorMessage,errorStack, the errorname/codewhen present, and the issue key —[factory] error {}during a dispatch doesn't even say which issue it belongs to.This is systemic, not one line
27 call sites in
factory.tsalone pass a raw error as the log metadata (grep -cE "logger\.(error|warn|info)\?\.\([^,]+, *(error|err)\)"). Every one has the same defect. Examples:Suggested approach:
#error()first — it's the loudest and it increments a metric.Errorcan never render as{}regardless of call site. That fixes all 27 without touching 27 lines, and prevents the 28th.Error/unknownas the metadata object.Why it matters beyond tidiness
#increment('errors')) — a metric you can't act on.[factory] error {}reads as something is broken to anyone running a dispatch, with zero way to tell whether it's benign teardown noise or a real failure. In practice it appears on every run (I've seen it on 4/4 clean dispatches that produced correct PRs), which trains operators to ignore an error-level log — the worst possible outcome for the one line that should mean "look here."internal reply dropped/failed to release ar-NN-reviewteardown failure is currently only diagnosable because that particular error class happens to serialize. A plainErrorin the same path would be invisible.Related: #67/#69 (reviewer-hang teardown — the errors that surface here), #82 (agent-name collision — diagnosed only because
resume-already-existswas logged with real fields).