diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c6e797..79feca6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## Unreleased + +### Bug fixes + +* Detect a panic header that is split across stderr reads + ## 1.3.4 (2021-08-10) ### Bug fixes diff --git a/panicwrap.go b/panicwrap.go index 9135c5b..493213e 100644 --- a/panicwrap.go +++ b/panicwrap.go @@ -264,7 +264,17 @@ func trackPanic(r io.Reader, w io.Writer, dur time.Duration, result chan<- strin []byte("panic:"), []byte("fatal error:"), } + // A header can start in one read and finish in the next. Hold back + // one byte less than the longest header so that tail can be joined + // with the following read before it is treated as ordinary output. + keep := 0 + for _, header := range panicHeaders { + if len(header)-1 > keep { + keep = len(header) - 1 + } + } panicType := -1 + var carry []byte tempBuf := make([]byte, 2048) for { @@ -288,17 +298,31 @@ func trackPanic(r io.Reader, w io.Writer, dur time.Duration, result chan<- strin var err error buf = tempBuf n, err = r.Read(buf) - if n <= 0 && err == io.EOF { + if n > 0 && panicTimer == nil && len(carry) > 0 { + merged := make([]byte, 0, len(carry)+n) + merged = append(merged, carry...) + merged = append(merged, buf[:n]...) + buf = merged + n = len(merged) + carry = nil + } + if n <= 0 && err != nil { if panicBuf.Len() > 0 { // We were tracking a panic, assume it was a panic // and return that as the result. result <- panicBuf.String() + } else if len(carry) > 0 { + w.Write(carry) } return } } + if n <= 0 { + continue + } + if panicTimer != nil { // We're tracking what we think is a panic right now. // If the timer ended, then it is not a panic. @@ -333,14 +357,22 @@ func trackPanic(r io.Reader, w io.Writer, dur time.Duration, result chan<- strin } } - // Flush to stderr what isn't a panic - w.Write(buf[0:flushIdx]) - if panicType == -1 { - // Not a panic so just continue along + // Not a panic. Hold back the tail so a header split across + // this read and the next one is still visible. + if n > keep { + w.Write(buf[0 : n-keep]) + carry = append([]byte(nil), buf[n-keep:n]...) + } else { + carry = append([]byte(nil), buf[:n]...) + } continue } + // Flush to stderr what isn't a panic + w.Write(buf[0:flushIdx]) + carry = nil + // We have a panic header. Write we assume is a panic os far. panicBuf.Write(buf[flushIdx:n]) panicTimer = time.After(dur) diff --git a/panicwrap_test.go b/panicwrap_test.go index 771e1eb..e55c418 100644 --- a/panicwrap_test.go +++ b/panicwrap_test.go @@ -3,6 +3,7 @@ package panicwrap import ( "bytes" "fmt" + "io" "os" "os/exec" "runtime" @@ -221,9 +222,9 @@ func TestHelperProcess(*testing.T) { case "panic-monitor": config := &WrapConfig{ - Handler: panicHandler, + Handler: panicHandler, HidePanic: true, - Monitor: true, + Monitor: true, } exitStatus, err := Wrap(config) @@ -396,9 +397,6 @@ func TestPanicWrap_concurrentMapWrite(t *testing.T) { } func TestPanicWrap_panicBoundary(t *testing.T) { - // TODO(mitchellh): panics are currently lost on boundaries - t.SkipNow() - stdout := new(bytes.Buffer) p := helperProcess("panic-boundary") @@ -408,11 +406,48 @@ func TestPanicWrap_panicBoundary(t *testing.T) { t.Fatalf("err: %s", err) } - if !strings.Contains(stdout.String(), "wrapped: 1015") { + // The child writes "pan" and then "ic: oh crap". + if !strings.Contains(stdout.String(), "wrapped: 14") { t.Fatalf("didn't wrap: %#v", stdout.String()) } } +func TestTrackPanic_splitHeader(t *testing.T) { + r, w := io.Pipe() + out := new(bytes.Buffer) + ch := make(chan string) + go trackPanic(r, out, time.Second, ch) + go func() { + w.Write([]byte("before pan")) + w.Write([]byte("ic: split\n")) + w.Close() + }() + got := <-ch + if got != "panic: split\n" { + t.Fatalf("panic text: %q", got) + } + if out.String() != "before " { + t.Fatalf("forwarded: %q", out.String()) + } +} + +func TestTrackPanic_plainTextFlushed(t *testing.T) { + r, w := io.Pipe() + out := new(bytes.Buffer) + ch := make(chan string) + go trackPanic(r, out, time.Second, ch) + go func() { + w.Write([]byte("hello world, not a panic")) + w.Close() + }() + if got := <-ch; got != "" { + t.Fatalf("panic text: %q", got) + } + if out.String() != "hello world, not a panic" { + t.Fatalf("forwarded: %q", out.String()) + } +} + func TestWrapped(t *testing.T) { stdout := new(bytes.Buffer)