Found during the adversarial review of #21
(#21 (comment)). Pre-existing,
not introduced by that PR.
JSONHandler.Handle discards the error from its write to stdout and returns nil unconditionally. slog.Handler.Handle is declared to return an error
precisely so a failing sink can be reported; returning nil tells log/slog
the record was delivered when it was not.
This surfaced in that review as a lint question — the line is written _, _ = fmt.Fprintln(...), which a newer golangci-lint flags and the repo's
pinned linter does not. Silencing the linter is not the fix and would bury the
defect. The defect is that a log line can vanish with nothing reporting it.
Definition of done
A failed write to the sink is returned from Handle rather than discarded, in
every handler that writes to a sink, not only JSONHandler.
The behaviour is decided deliberately for the multiplex case: one child
failing must not silently suppress delivery to its siblings, and the caller
must still learn that something failed.
A test drives a handler whose sink returns an error and asserts the error
reaches the caller. This requires the sink to be injectable; if it is not
today, make it so rather than skipping the test.
Whatever is decided is written down in the README alongside the handler
descriptions, since "logging can fail and here is how you find out" is
caller-facing behaviour.
The repo's full check is green.
Implementation requirements
Do not paper over this by writing to a buffer that cannot fail.
Do not reintroduce any path back into the stdlib log package: that is the v1.0.0 deadlock in #18, and TestJSONHandlerDeadlock guards it.
Landing commit title must end with (closes #<this issue>).
Found during the adversarial review of
https://git.eeqj.de/sneak/simplelog/pulls/21
(https://git.eeqj.de/sneak/simplelog/pulls/21#issuecomment-53518). Pre-existing,
not introduced by that PR.
`JSONHandler.Handle` discards the error from its write to stdout and returns
`nil` unconditionally. `slog.Handler.Handle` is declared to return an `error`
precisely so a failing sink can be reported; returning `nil` tells `log/slog`
the record was delivered when it was not.
This surfaced in that review as a lint question — the line is written
`_, _ = fmt.Fprintln(...)`, which a newer golangci-lint flags and the repo's
pinned linter does not. Silencing the linter is not the fix and would bury the
defect. The defect is that a log line can vanish with nothing reporting it.
## Definition of done
- A failed write to the sink is returned from `Handle` rather than discarded, in
every handler that writes to a sink, not only `JSONHandler`.
- The behaviour is decided deliberately for the multiplex case: one child
failing must not silently suppress delivery to its siblings, and the caller
must still learn that something failed.
- A test drives a handler whose sink returns an error and asserts the error
reaches the caller. This requires the sink to be injectable; if it is not
today, make it so rather than skipping the test.
- Whatever is decided is written down in the README alongside the handler
descriptions, since "logging can fail and here is how you find out" is
caller-facing behaviour.
- The repo's full check is green.
## Implementation requirements
- Do not paper over this by writing to a buffer that cannot fail.
- Do not reintroduce any path back into the stdlib `log` package: that is the
`v1.0.0` deadlock in https://git.eeqj.de/sneak/simplelog/issues/18, and
`TestJSONHandlerDeadlock` guards it.
- Landing commit title must end with ` (closes #<this issue>)`.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Found during the adversarial review of
#21
(#21 (comment)). Pre-existing,
not introduced by that PR.
JSONHandler.Handlediscards the error from its write to stdout and returnsnilunconditionally.slog.Handler.Handleis declared to return anerrorprecisely so a failing sink can be reported; returning
niltellslog/slogthe record was delivered when it was not.
This surfaced in that review as a lint question — the line is written
_, _ = fmt.Fprintln(...), which a newer golangci-lint flags and the repo'spinned linter does not. Silencing the linter is not the fix and would bury the
defect. The defect is that a log line can vanish with nothing reporting it.
Definition of done
Handlerather than discarded, inevery handler that writes to a sink, not only
JSONHandler.failing must not silently suppress delivery to its siblings, and the caller
must still learn that something failed.
reaches the caller. This requires the sink to be injectable; if it is not
today, make it so rather than skipping the test.
descriptions, since "logging can fail and here is how you find out" is
caller-facing behaviour.
Implementation requirements
logpackage: that is thev1.0.0deadlock in #18, andTestJSONHandlerDeadlockguards it.(closes #<this issue>).clawbot referenced this issue2026-09-03 15:06:18 +02:00