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
3 changes: 3 additions & 0 deletions internal/sandbox/sandbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,9 @@ func ensureProviderArgs(ctx context.Context, name string, args, updateArgs, extr
if lastErr == nil {
return nil
}
if err := ctx.Err(); err != nil {
return err
}
// Retry only on the transient concurrency errors.
if !isTransientProviderErr(lastErr) {
return lastErr
Expand Down
37 changes: 24 additions & 13 deletions internal/sandbox/sandbox_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2204,38 +2204,49 @@ exit 0
}

// TestEnsureProvider_RetryCancelledByContext verifies that context
// cancellation during the retry backoff sleep causes EnsureProvider to
// return the context error instead of continuing to retry.
// cancellation during a provider creation attempt returns the context error
// instead of the process error produced when CommandContext kills openshell.
func TestEnsureProvider_RetryCancelledByContext(t *testing.T) {
dir := t.TempDir()
markerDir := filepath.Join(dir, "markers")
require.NoError(t, os.MkdirAll(markerDir, 0o755))

// Fake openshell: always fails with the transient error so the
// retry loop never succeeds on its own.
// Fake openshell: record the attempt, then block until context cancellation
// kills the process. The busy loop uses only shell builtins so PATH can
// contain just this fake binary.
script := fmt.Sprintf(`#!/bin/sh
if [ "$2" = "create" ]; then
echo x > "%s/attempt.$$"
echo "Error: × unsupported provider type or profile: test" >&2
exit 1
while :; do :; done
fi
exit 0
`, markerDir)
fakePath := filepath.Join(dir, "openshell")
require.NoError(t, os.WriteFile(fakePath, []byte(script), 0o755))
t.Setenv("PATH", dir)

// Cancel the context shortly after the first attempt so the select
// picks up ctx.Done() during the backoff sleep.
ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond)
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
errCh := make(chan error, 1)
go func() {
errCh <- EnsureProvider(ctx, "p", "custom", nil, nil, false)
}()

err := EnsureProvider(ctx, "p", "custom", nil, nil, false)
require.Eventually(t, func() bool {
entries, err := os.ReadDir(markerDir)
return err == nil && len(entries) == 1
}, 5*time.Second, 10*time.Millisecond, "provider creation should start")
cancel()

var err error
select {
case err = <-errCh:
case <-time.After(5 * time.Second):
t.Fatal("EnsureProvider did not return after cancellation")
}
require.Error(t, err)
assert.ErrorIs(t, err, context.DeadlineExceeded, "should return context error when cancelled during retry sleep")
assert.ErrorIs(t, err, context.Canceled, "should return context error when creation is cancelled")

// Should have made only 1 attempt before the context expired during
// the backoff sleep.
entries, readErr := os.ReadDir(markerDir)
require.NoError(t, readErr)
assert.Len(t, entries, 1, "should stop retrying when context is cancelled")
Expand Down
Loading