Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/specs/reporter-package.md
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ JSON Lines output (one `ProgressEvent` per line) to an `io.Writer`.
- The mutex lock covers both timestamp generation and encoding (including the fallback encode below)
- **Encode failure:** if `json.Encoder.Encode` fails because the caller-supplied `Details` value is not JSON-encodable (`*json.UnsupportedTypeError` for a func or channel, `*json.UnsupportedValueError` for NaN/±Inf, `*json.MarshalerError` for a failing `json.Marshaler`, matched with `errors.As`), `emit()` re-encodes the same event with `Details` replaced by `map[string]any{"encoding_error": err.Error()}` — `Type`, `Timestamp`, `Message`, and step fields are unchanged — so the line (in particular the terminal `complete` event) still reaches the stream as a valid `ProgressEvent`. `json.Encoder` marshals the whole value before writing, so a failed encode leaves nothing on the wire and exactly one line is emitted. A `Details` value's `json.Marshaler` implementation that panics instead of returning an error is handled identically: `emit()` recovers the panic and routes it through the same encode-failure fallback
- **Writer failure:** errors returned by the underlying `io.Writer` are silent to callers; under the same mutex, the reporter latches the first primary or fallback writer error and makes every later `emit()` a no-op, so a partial JSON record cannot be followed by misleading apparently valid records
- **Writer panic:** a panic raised by the underlying `io.Writer`'s `Write` method (primary encode or fallback encode) is caught by `writerPanicGuard`, a wrapper installed around the writer at construction, and converted into an ordinary error before it reaches `emit()`. It is therefore treated exactly like a returned writer error — never mislabeled as a `Details` encoding error — and latches the same failed state. Marshaling `Details` happens entirely in memory before `Write` is ever called, so this guard cannot observe, and never interferes with, a panicking `Details` `json.Marshaler`

### NoopReporter (`reporter/noop.go`)

Expand Down
26 changes: 24 additions & 2 deletions reporter/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,27 @@ type JSONReporter struct {
// either a literal nil or an io.Writer holding a nil concrete value — is
// treated as io.Discard so reporting remains silent and non-panicking.
func NewJSONReporter(w io.Writer) *JSONReporter {
return &JSONReporter{encoder: json.NewEncoder(discardIfNil(w))}
return &JSONReporter{encoder: json.NewEncoder(writerPanicGuard{w: discardIfNil(w)})}
}

// writerPanicGuard wraps an io.Writer and converts a panic raised by its
// Write method into an ordinary error, so a transport failure below the JSON
// encoder is reported through the same path as a writer that merely returns
// an error, and cannot escape emit() as a panic. Marshaling a Details value
// happens entirely before Write is ever called, so this guard never sees —
// and never mislabels — a panicking json.Marshaler; that is handled by
// encodeRecover instead.
type writerPanicGuard struct {
w io.Writer
}

func (g writerPanicGuard) Write(p []byte) (n int, err error) {
defer func() {
if r := recover(); r != nil {
err = fmt.Errorf("reporter: writer panicked: %v", r)
}
}()
return g.w.Write(p)
}

// emit stamps the event and writes it as one JSON line. When the event
Expand All @@ -31,7 +51,9 @@ func NewJSONReporter(w io.Writer) *JSONReporter {
// {"encoding_error": "<reason>"} so the line — in particular the terminal
// `complete` event — still reaches the stream as a valid ProgressEvent. A
// panicking Marshaler is handled the same way as one that returns an error.
// Writer (I/O) errors are discarded: a progress stream has no channel to
// Writer (I/O) errors — including a panic raised by the underlying
// io.Writer's Write method, converted to an error by writerPanicGuard before
// it reaches this method — are discarded: a progress stream has no channel to
// report its own transport failure and must never abort the caller. After the
// first writer error, later events are dropped so they cannot follow a partial
// JSON record on the same stream.
Expand Down
49 changes: 49 additions & 0 deletions reporter/json_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,55 @@ func TestJSONReporter_FallbackWriterErrorStopsLaterEvents(t *testing.T) {
}
}

func TestJSONReporter_WriterPanicIsSilent(t *testing.T) {
w := &panickingWriter{}
r := NewJSONReporter(w)

// A normal event reaches Write directly during the primary encode.
r.Complete("done", map[string]string{"device": "/dev/sda"})

if !r.failed {
t.Error("reporter did not latch primary writer panic")
}
if w.calls != 1 {
t.Fatalf("writer called %d times, want 1", w.calls)
}

r.Message("must not be attempted")
if w.calls != 1 {
t.Errorf("writer called %d times after latch, want still 1", w.calls)
}
}

func TestJSONReporter_FallbackWriterPanicIsSilent(t *testing.T) {
w := &panickingWriter{}
r := NewJSONReporter(w)

// Marshaling an unencodable Details value fails entirely in memory, so
// the primary encode never calls Write; the panic can only be observed
// in the fallback encode after Details is replaced.
r.Complete("done", func() {})

if !r.failed {
t.Error("reporter did not latch fallback writer panic")
}
if w.calls != 1 {
t.Fatalf("writer called %d times, want 1", w.calls)
}
}

// panickingWriter is an io.Writer whose Write always panics, simulating a
// transport failure that surfaces as a panic (for example a closed pipe or
// broken connection) rather than a returned error.
type panickingWriter struct {
calls int
}

func (w *panickingWriter) Write([]byte) (int, error) {
w.calls++
panic("writer panicked")
}

func TestJSONReporter_NilWriterIsSilent(t *testing.T) {
r := NewJSONReporter(nil)

Expand Down