From 35928f66c1b50e424d649ee6bb11a2d503c74764 Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Wed, 16 Sep 2026 08:44:48 +0200 Subject: [PATCH] fix(up): drop benign context.Canceled noise from the final report The new TestUpAttachedTerminatesOnExternalStop (#13990) occasionally observed `up` exit 1 instead of 0 after an external stop/down cancels a service's restart backoff. The cause: once the monitor detects termination and cancels globalCtx, a log-streaming goroutine still blocked on logs?follow=1 for the backoff container gets a context.Canceled error, which appendErr collected unfiltered into the final errors.Join. That race could only surface once #13990 made the monitor loop actually return in this scenario -- before that fix, up hung forever and never reached this code path. appendErr now drops an error that is nothing more than fallout from our own shutdown (errors.Is(err, context.Canceled) once u.globalCtx.Err() is set), the same way a SIGINT-triggered teardown's residual goroutine errors are already expected to be harmless. A genuine error is still reported regardless of shutdown state. Signed-off-by: Nicolas De Loof --- pkg/compose/up.go | 19 +++++++++++++++---- pkg/compose/up_test.go | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/pkg/compose/up.go b/pkg/compose/up.go index 0beb3637359..c08075ec988 100644 --- a/pkg/compose/up.go +++ b/pkg/compose/up.go @@ -227,12 +227,23 @@ func (s *composeService) setupNavigationMenu(ctx context.Context, options *api.U return formatter.NewKeyboardManager(isDockerDesktopActive, isLogsViewEnabled, signalChan), kEvents, nil } +// appendErr records err for the final report, unless it is nothing more than +// fallout from our own shutdown: once u.globalCtx is canceled (monitor +// detecting termination, SIGINT/SIGTERM, or an earlier setup failure), the +// in-flight goroutines it carries (log/attach streaming in particular) get a +// context.Canceled error that reports no real failure and must not turn a +// clean exit into a non-zero one (#13985). func (u *upSession) appendErr(err error) { - if err != nil { - u.mu.Lock() - u.errs = append(u.errs, err) - u.mu.Unlock() + if err == nil { + return + } + if errors.Is(err, context.Canceled) && u.globalCtx.Err() != nil { + logrus.Debugf("ignoring canceled error after shutdown: %v", err) + return } + u.mu.Lock() + u.errs = append(u.errs, err) + u.mu.Unlock() } // runEventLoop reacts to cancellation, SIGINT/SIGTERM and keyboard input until diff --git a/pkg/compose/up_test.go b/pkg/compose/up_test.go index f38a9e1aeeb..9cdfa49c825 100644 --- a/pkg/compose/up_test.go +++ b/pkg/compose/up_test.go @@ -17,6 +17,9 @@ package compose import ( + "context" + "errors" + "fmt" "testing" "gotest.tools/v3/assert" @@ -102,3 +105,35 @@ func TestShouldFollowStartEvent(t *testing.T) { }) } } + +// TestAppendErrDropsCancellationAfterShutdown is the #13985 follow-up: once +// our own shutdown has canceled globalCtx (monitor detecting termination, +// SIGINT/SIGTERM, ...), a lingering goroutine (log/attach streaming) racing +// that cancellation reports a context.Canceled error carrying no real +// failure. appendErr must drop it instead of turning a clean exit into a +// non-zero one, while still reporting any other, genuine error. +func TestAppendErrDropsCancellationAfterShutdown(t *testing.T) { + ctx, cancel := context.WithCancel(t.Context()) + u := &upSession{globalCtx: ctx} + + u.appendErr(errors.New("boom")) + assert.Equal(t, len(u.errs), 1) + + cancel() + + u.appendErr(fmt.Errorf("streaming logs: %w", context.Canceled)) + assert.Equal(t, len(u.errs), 1, "a context-canceled error after our own shutdown must be dropped") + + u.appendErr(errors.New("a real, unrelated failure")) + assert.Equal(t, len(u.errs), 2, "a genuine error occurring after shutdown must still be reported") +} + +// TestAppendErrKeepsCancellationBeforeShutdown pins the guard on +// globalCtx.Err(): a context.Canceled error must still be reported if it +// didn't come from our own globalCtx being canceled. +func TestAppendErrKeepsCancellationBeforeShutdown(t *testing.T) { + u := &upSession{globalCtx: t.Context()} + + u.appendErr(context.Canceled) + assert.Equal(t, len(u.errs), 1) +}