fix(cli): retry transient connection failures during SSH setup (#24010)

When `coder ssh` connects to a workspace after laptop wake, DNS or the
control plane may be briefly unavailable. Previously this caused an
immediate failure, which VS Code Remote SSH classified as permanent
("Reload Window").

Wrap each network step (workspace resolution, template version fetch,
agent connection info, Coder Connect dial, tailnet dial) with
`retryWithInterval` so transient errors (DNS, connection refused, 5xx)
are retried individually. Non-retryable errors (auth, 404) and context
cancellation stop immediately. Data transfer is never retried.
This commit is contained in:
Ehab Younes
2026-04-08 00:59:10 +03:00
committed by GitHub
parent 48bc215f20
commit d00f148b76
2 changed files with 221 additions and 17 deletions
+129 -1
View File
@@ -5,7 +5,9 @@ import (
"fmt"
"io"
"net"
"net/http"
"net/url"
"os"
"sync"
"testing"
"time"
@@ -226,6 +228,21 @@ func TestCloserStack_Timeout(t *testing.T) {
testutil.TryReceive(ctx, t, closed)
}
func TestCloserStack_PushAfterClose_ConnClosed(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitShort)
logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug)
uut := newCloserStack(ctx, logger, quartz.NewMock(t))
uut.close(xerrors.New("canceled"))
closes := new([]*fakeCloser)
fc := &fakeCloser{closes: closes}
err := uut.push("conn", fc)
require.Error(t, err)
require.Equal(t, []*fakeCloser{fc}, *closes, "should close conn on failed push")
}
func TestCoderConnectStdio(t *testing.T) {
t.Parallel()
@@ -254,7 +271,7 @@ func TestCoderConnectStdio(t *testing.T) {
stdioDone := make(chan struct{})
go func() {
err = runCoderConnectStdio(ctx, ln.Addr().String(), clientOutput, serverInput, stack)
err = runCoderConnectStdio(ctx, ln.Addr().String(), clientOutput, serverInput, stack, logger)
assert.NoError(t, err)
close(stdioDone)
}()
@@ -448,3 +465,114 @@ func Test_getWorkspaceAgent(t *testing.T) {
assert.Contains(t, err.Error(), "available agents: [clark krypton zod]")
})
}
func TestIsRetryableError(t *testing.T) {
t.Parallel()
tests := []struct {
name string
err error
retryable bool
}{
{"Nil", nil, false},
{"ContextCanceled", context.Canceled, false},
{"ContextDeadlineExceeded", context.DeadlineExceeded, false},
{"WrappedContextCanceled", xerrors.Errorf("wrapped: %w", context.Canceled), false},
{"DNSError", &net.DNSError{Err: "no such host", Name: "example.com", IsNotFound: true}, true},
{"OpError", &net.OpError{Op: "dial", Net: "tcp", Err: &os.SyscallError{}}, true},
{"WrappedDNSError", xerrors.Errorf("connect: %w", &net.DNSError{Err: "no such host", Name: "example.com"}), true},
{"SDKError_500", codersdk.NewTestError(http.StatusInternalServerError, "GET", "/api"), true},
{"SDKError_502", codersdk.NewTestError(http.StatusBadGateway, "GET", "/api"), true},
{"SDKError_503", codersdk.NewTestError(http.StatusServiceUnavailable, "GET", "/api"), true},
{"SDKError_401", codersdk.NewTestError(http.StatusUnauthorized, "GET", "/api"), false},
{"SDKError_403", codersdk.NewTestError(http.StatusForbidden, "GET", "/api"), false},
{"SDKError_404", codersdk.NewTestError(http.StatusNotFound, "GET", "/api"), false},
{"GenericError", xerrors.New("something went wrong"), false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
assert.Equal(t, tt.retryable, isRetryableError(tt.err))
})
}
}
func TestRetryWithInterval(t *testing.T) {
t.Parallel()
const interval = time.Millisecond
const maxAttempts = 3
dnsErr := &net.DNSError{Err: "no such host", Name: "example.com", IsNotFound: true}
logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug)
t.Run("Succeeds_FirstTry", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitShort)
attempts := 0
err := retryWithInterval(ctx, logger, interval, maxAttempts, func() error {
attempts++
return nil
})
require.NoError(t, err)
assert.Equal(t, 1, attempts)
})
t.Run("Succeeds_AfterTransientFailures", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitShort)
attempts := 0
err := retryWithInterval(ctx, logger, interval, maxAttempts, func() error {
attempts++
if attempts < 3 {
return dnsErr
}
return nil
})
require.NoError(t, err)
assert.Equal(t, 3, attempts)
})
t.Run("Stops_NonRetryableError", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitShort)
attempts := 0
err := retryWithInterval(ctx, logger, interval, maxAttempts, func() error {
attempts++
return xerrors.New("permanent failure")
})
require.ErrorContains(t, err, "permanent failure")
assert.Equal(t, 1, attempts)
})
t.Run("Stops_MaxAttemptsExhausted", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitShort)
attempts := 0
err := retryWithInterval(ctx, logger, interval, maxAttempts, func() error {
attempts++
return dnsErr
})
require.Error(t, err)
assert.Equal(t, maxAttempts, attempts)
})
t.Run("Stops_ContextCanceled", func(t *testing.T) {
t.Parallel()
ctx, cancel := context.WithCancel(context.Background())
attempts := 0
err := retryWithInterval(ctx, logger, interval, maxAttempts, func() error {
attempts++
cancel()
return dnsErr
})
require.Error(t, err)
assert.Equal(t, 1, attempts)
})
}