[junior]

Point 1 of the review of #41. Since #41, os.Stderr is a pipe while the TUI runs. When the TUI panics, Bubble Tea 1.3.10 prints the panic's value to stdout but the stack trace with debug.PrintStack(), which writes to os.Stderr: the trace went into the pipe, which kept only its last line.

The tap now also keeps the last 64 KiB it read, line by line. When Run returns tea.ErrProgramPanic, main waits up to a second for the pipe to go quiet, closes it, gives the reader up to a second to finish (a credential plugin that is still running holds the pipe open too), and writes those bytes to the terminal ahead of its k10s: ... line. The wait is for a panic in a command's goroutine: Bubble Tea writes that trace from the goroutine after ending the program, and Run does not wait for it. Both panics Bubble Tea recovers are covered: on its event loop, and in a command's goroutine. A normal exit is unchanged.

Tests: TestBubbleTeaPanicStackTracesReachTheTail runs a Bubble Tea program that panics each way with the tap in place and checks that both stack traces are in the tail; TestDrainStderrGivesBackWhatWasWritten and TestStderrTailKeepsTheNewestBytes cover the tail itself, and TestTUIFailedPutsTheStackTraceFirst covers main's side, with a stand-in for the pipe, which a process captures only once. The tail tests fail with a tap that keeps nothing, and the main test fails without the write. gofmt, vet and go test -short ./... pass on Go 1.26.11 and 1.27.4, and so does -race on ., internal/k8s and internal/ui. TestBubbleTeaPanicStackTracesReachTheTail failed now and then without the wait, as pierreneter found; with it, it passed 300 runs of 300 on darwin/arm64 and 500 of 500 on linux/amd64.

[junior]

@pierreneter, could you take a look at this one when you have a moment? It's the stack-trace fix from the #41 review, and the last change for v0.1.10: #39 is in and you've approved the notes in #43, so once this lands I'll merge the notes and tag the release.

[pierreneter]

Requesting changes: TestBubbleTeaPanicStackTracesReachTheTail is flaky, and main has the same race

Thanks for this. The approach is right: debug.PrintStack() writes to the os.Stderr variable, so the tail does catch the trace. The goroutine-panic path has a race, though.

Repro

go test -c -o k8s.test ./internal/k8s
for i in $(seq 100); do
  ./k8s.test -test.run 'TestBubbleTeaPanicStackTracesReachTheTail$' >/dev/null 2>&1 || echo fail
done | wc -l

It failed 25 times out of 100 for me on darwin/arm64 with Go 1.27.3. It also failed on my first plain go test -short ./... run. CI passing looks like luck.

credplugin_test.go:343: no k8s.panicInCommand( in the tail:
    goroutine 299 [running]:
    ...
    github.com/p10node/k10s/internal/k8s.panicsInUpdate.Update(...)

The tail holds only the Update trace. The command trace never reached it.

Cause

When a command panics, Bubble Tea 1.3.10 handles it in recoverFromGoPanic (tea.go:852). It sends ErrProgramPanic on p.errs and calls p.cancel() first. Only after that does it call fmt.Printf and debug.PrintStack(). Run does not wait for command goroutines (see the comment at tea.go:349), so it can return before the trace is written.

After that, drain closes the write end straight away. If the goroutine's os.Stderr.Write lands after the Close, it fails with os.ErrClosed and the trace is lost.

This is not only a test problem. main does the same thing: Run returns, then ReleaseStderr, then tuiFailed, then drain, which closes the pipe. In production the window is smaller because Run restores a real TTY before it returns, but the race is still there.

Suggested fix

Wait for the pipe to go quiet before closing it:

type stderrTap struct {
	// ...
	done chan struct{} // closed when the reader stops
	read int64         // bytes read so far
}

// in the reader loop, after t.keep(raw):
t.read += int64(len(raw)) + 1

func (t *stderrTap) drain(wait time.Duration) []byte {
	if t.pipe == nil {
		return nil
	}
	// A command goroutine Bubble Tea left behind may still be writing its
	// stack trace after Run returned: let the pipe go quiet before closing it.
	deadline := time.Now().Add(wait)
	for prev := int64(-1); time.Now().Before(deadline); time.Sleep(50 * time.Millisecond) {
		t.mu.Lock()
		n := t.read
		t.mu.Unlock()
		if n == prev {
			break
		}
		prev = n
	}
	_ = t.pipe.Close()
	select {
	case <-t.done:
	case <-time.After(wait):
	}
	t.mu.Lock()
	defer t.mu.Unlock()
	return append([]byte(nil), t.tail...)
}

With this change the test passed 200 out of 200 runs, and go test -race -short -count=3 ./internal/k8s . passes. A panic exit now takes at least about 100 ms longer, which is fine on that path. Use a different approach if you prefer; the requirement is that a late writer still gets its trace in.

Nits (non-blocking)

• "Byte for byte" is not quite accurate. bufio.ScanLines drops a \r before \n, and keep adds a \n to a final line that had none. Stack traces are unaffected, so either soften the wording in the comments and PR body or leave it.
• For a command panic, Bubble Tea prints the panic value (Caught goroutine panic: ...) to stdout while the alt screen is still on, so the value is erased when the screen is restored. The user sees the trace but not the message. That is Bubble Tea's behaviour and outside this PR's scope; it might be worth a follow-up issue.

Please re-run the loop above after the fix before re-requesting review.

[junior]

Thanks for digging into this, and for the fix! You're right: recoverFromGoPanic cancels before it prints, and Run doesn't wait for the command's goroutine, so drain could close the pipe under it.

I took your drain change: it now waits until nothing new has come through the pipe for 50 ms, up to the wait, and then closes it. Your loop, before and after:

• darwin/arm64, Go 1.27.4: 22 of 100 failed before, 0 of 300 after
• linux/amd64, Go 1.27.4: 4 of 100 failed before, 0 of 500 after

go test -race -short -count=3 ./internal/k8s . passes too, and the comments and the description no longer say byte for byte.

And agreed on the panic value: Bubble Tea prints it on the alt screen, so it's gone with it, and that's outside this PR, as you say.

[pierreneter]

Thanks @junior, the quiet-wait fix holds up: 0 failures in 200 runs of the loop on darwin/arm64 and the trace shows for both panic kinds.