Two writers bypass internal/logger: fx's console logger and the Go runtime #183
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Found by the writer sweep #178 required. Deliberately NOT milestoned 1.0.0 — neither writer is client-sized, so neither is the log-amplification class that #146, #176 and #178 were milestoned for.
Title and body corrected 2026-08-18. As filed this said three writers and led with
net/http'sErrorLog. The review of #182 measured the set at fd level instead of reading it, and disproved that. See the comment below; the corrected set is what follows. The original claim was mine and was wrong.The writers, as measured
fxconsole loggerfx.WithLoggeranywhere, sofx.Newbuildsfxlog.DefaultLogger(os.Stderr)directly.net/httpnilErrorLogslog.SetDefault(internal/logger/logger.go:80) redirects thelogpackage's default logger into the installed handler, so these arrive as ordinary JSON records atINFO. Struck from this issue.Two further counts in the original body were also wrong and are corrected here: there are six
os.Stdout/os.Stderrreferences outside_test.gofiles, not zero-outside-internal/logger— four ininternal/logger/logger.goand two ininternal/database/testing.go; and there are fivepaniccalls in the tree, not three (internal/delivery/ssrf.go:64,internal/database/password.go:140,:145,:308,:316). All five are invariant guards over constants andcrypto/rand, so the conclusion held even though the count did not.Not in scope here
The handler-panic path is #187, which is milestoned 1.0.0. chi's
Recovererpanics inside its own stack pretty-printer before writing anything, the second panic escapes, and the connection is dropped — a real defect the original sweep missed entirely while chasingErrorLog. Do not fix it here.Why this is still worth fixing
Not for amplification — for coherence. An operator who configures JSON logging and ships it to a collector gets a different shape on a second file descriptor for the startup graph, and no shape at all for a fatal error. The
fxgraph dump is also the one line an operator reads on every restart.Definition of done
fx.WithLoggerinstalls anfxevent.Loggeroverinternal/logger. Choose the level deliberately: the graph dump is useful once at startup and noise thereafter.fxlifecycle events arrive as structured records on the configured handler.Implementation requirements
next, PR based onnext, single commit, title ending(closes #N).TODO.md(see #112).make bootstrapin a fresh clone before gating — browser assets are fetched at build time, andmake lintneeds Docker.make checkplus the Docker lint path with the cache defeated. All linting runs in Docker, never on the host. Clean up every container and image you start; never prune.Correcting this issue: the enumeration in the body above is wrong. It came from the writer sweep in #182, and the independent review of that PR disproved two of its three claims by measurement. I filed it on that basis, so the error is mine to fix.
What is actually true:
net/http'sErrorLogis NOT outsideinternal/logger, and NOT on stderr.internal/logger/logger.go:80callsslog.SetDefault, which internally doeslog.SetOutput(&handlerWriter{...}). So the nilErrorLogfalls through tolog.Default(), which is already rerouted through the service's own handler — and lands on stdout at INFO. Measured, not read. It is therefore structured and levelled already; what is arguably wrong is only the level and the stream, which is a much smaller thing than this issue claims.internal/server/routes.go:32installs chi'smiddleware.Recoverer, whosePrintPrettyStackwrites toos.Stderrdirectly. That is what really catches a handler panic — the case this issue's body used as its motivating example fornet/http. The review of #180 had already named it, and I did not carry it across.fxconsole logger and the Go runtime remain as described. The runtimepaniccount is five, not three, all invariant guards.Net: the shape of the issue survives — there are writers that bypass or diverge from
internal/logger, and a panic stack still lands unstructured on stderr — but the specific writer to fix is chi'sRecoverer, notnet/http'sErrorLog, and theErrorLogitem shrinks to a level-and-stream question.Still NOT milestoned
1.0.0: none of these is client-sized, so none is the log-amplification class #146, #176 and #178 were milestoned for.Whoever picks this up should re-derive the writer list from the tree rather than trusting either the body or this comment — that is twice now this enumeration has been stated wrong from reading rather than measuring.
clawbot referenced this issue2026-08-18 03:25:33 +02:00
Three writers bypass internal/logger entirely: net/http's ErrorLog, fx's console logger, and the Go runtimeto Two writers bypass internal/logger: fx's console logger and the Go runtime