fix(remote): close bootstrap pipe on start failure
- commit
- 72cf674e2efa29cba3a34546d30ed754c1c44b1a
- author
- brice · · 2026-07-06 21:47 UTC
- parent
- 2f5b15c
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +26 -13
M internal/adapters/dgram/ssh_dialer.go +1 -0
| @@ -97,6 +97,7 @@ func (d RemoteDialer) Dial(ctx context.Context) (ports.Transport, error) { | |||
| 97 | 97 | return nil, udpUnavailable("bootstrap stdout", err, &stderr) | |
| 98 | 98 | } | |
| 99 | 99 | if err := proc.Start(); err != nil { | |
| 100 | + | _ = stdout.Close() | |
| 100 | 101 | return nil, udpUnavailable("start bootstrap", err, &stderr) | |
| 101 | 102 | } | |
| 102 | 103 | cleanup := true | |
| @@ -97,6 +97,7 @@ func (d RemoteDialer) Dial(ctx context.Context) (ports.Transport, error) { | |||||
| 97 | return nil, udpUnavailable("bootstrap stdout", err, &stderr) | 97 | return nil, udpUnavailable("bootstrap stdout", err, &stderr) | ||
| 98 | } | 98 | } | ||
| 99 | if err := proc.Start(); err != nil { | 99 | if err := proc.Start(); err != nil { | ||
| 100 | + | _ = stdout.Close() | |||
| 100 | return nil, udpUnavailable("start bootstrap", err, &stderr) | 101 | return nil, udpUnavailable("start bootstrap", err, &stderr) | ||
| 101 | } | 102 | } | ||
| 102 | cleanup := true | 103 | cleanup := true | ||
M internal/adapters/dgram/ssh_dialer_test.go +24 -12
| @@ -21,6 +21,13 @@ type fakeBootstrapProcess struct { | |||
| 21 | 21 | waited bool | |
| 22 | 22 | } | |
| 23 | 23 | ||
| 24 | + | type closeTrackReadCloser struct { | |
| 25 | + | io.Reader | |
| 26 | + | closed *bool | |
| 27 | + | } | |
| 28 | + | ||
| 29 | + | func (r closeTrackReadCloser) Close() error { *r.closed = true; return nil } | |
| 30 | + | ||
| 24 | 31 | func (p *fakeBootstrapProcess) StdoutPipe() (io.ReadCloser, error) { return p.stdout, nil } | |
| 25 | 32 | func (p *fakeBootstrapProcess) Start() error { return p.startErr } | |
| 26 | 33 | func (p *fakeBootstrapProcess) Kill() error { p.killed = true; return nil } | |
| @@ -43,22 +50,24 @@ func withListenUDP(t *testing.T, fn func(context.Context) (net.PacketConn, error | |||
| 43 | 50 | func TestRemoteDialerUDPBootstrapFailures(t *testing.T) { | |
| 44 | 51 | key := base64.StdEncoding.EncodeToString(make([]byte, pdgram.KeySize)) | |
| 45 | 52 | tests := []struct { | |
| 46 | - | name string | |
| 47 | - | stdout string | |
| 48 | - | start error | |
| 49 | - | listen error | |
| 50 | - | want string | |
| 51 | - | killed bool | |
| 52 | - | waited bool | |
| 53 | + | name string | |
| 54 | + | stdout string | |
| 55 | + | start error | |
| 56 | + | listen error | |
| 57 | + | want string | |
| 58 | + | killed bool | |
| 59 | + | waited bool | |
| 60 | + | wantStdoutClosed bool | |
| 53 | 61 | }{ | |
| 54 | - | {name: "start failure", start: errors.New("ssh missing"), want: "vev: remote UDP transport unavailable: start bootstrap: ssh missing"}, | |
| 55 | - | {name: "malformed readiness", stdout: "hello\n", want: "malformed readiness line", killed: true, waited: true}, | |
| 56 | - | {name: "bootstrap wait failure", stdout: "VEV-UDP 4444 " + key + "\n", want: "wait bootstrap: exit status 255", waited: true}, | |
| 57 | - | {name: "packet listen failure", stdout: "VEV-UDP 4444 " + key + "\n", listen: errors.New("bind denied"), want: "listen UDP: bind denied", waited: true}, | |
| 62 | + | {name: "start failure", start: errors.New("ssh missing"), want: "vev: remote UDP transport unavailable: start bootstrap: ssh missing", wantStdoutClosed: true}, | |
| 63 | + | {name: "malformed readiness", stdout: "hello\n", want: "malformed readiness line", killed: true, waited: true, wantStdoutClosed: true}, | |
| 64 | + | {name: "bootstrap wait failure", stdout: "VEV-UDP 4444 " + key + "\n", want: "wait bootstrap: exit status 255", waited: true, wantStdoutClosed: true}, | |
| 65 | + | {name: "packet listen failure", stdout: "VEV-UDP 4444 " + key + "\n", listen: errors.New("bind denied"), want: "listen UDP: bind denied", waited: true, wantStdoutClosed: true}, | |
| 58 | 66 | } | |
| 59 | 67 | for _, tt := range tests { | |
| 60 | 68 | t.Run(tt.name, func(t *testing.T) { | |
| 61 | - | proc := &fakeBootstrapProcess{stdout: io.NopCloser(strings.NewReader(tt.stdout)), startErr: tt.start} | |
| 69 | + | stdoutClosed := false | |
| 70 | + | proc := &fakeBootstrapProcess{stdout: closeTrackReadCloser{Reader: strings.NewReader(tt.stdout), closed: &stdoutClosed}, startErr: tt.start} | |
| 62 | 71 | if tt.name == "bootstrap wait failure" { | |
| 63 | 72 | proc.waitErr = errors.New("exit status 255") | |
| 64 | 73 | } | |
| @@ -79,6 +88,9 @@ func TestRemoteDialerUDPBootstrapFailures(t *testing.T) { | |||
| 79 | 88 | if proc.waited != tt.waited { | |
| 80 | 89 | t.Fatalf("waited=%v, want %v", proc.waited, tt.waited) | |
| 81 | 90 | } | |
| 91 | + | if stdoutClosed != tt.wantStdoutClosed { | |
| 92 | + | t.Fatalf("stdout closed=%v, want %v", stdoutClosed, tt.wantStdoutClosed) | |
| 93 | + | } | |
| 82 | 94 | }) | |
| 83 | 95 | } | |
| 84 | 96 | } | |
| @@ -21,6 +21,13 @@ type fakeBootstrapProcess struct { | |||||
| 21 | waited bool | 21 | waited bool | ||
| 22 | } | 22 | } | ||
| 23 | 23 | ||||
| 24 | + | type closeTrackReadCloser struct { | |||
| 25 | + | io.Reader | |||
| 26 | + | closed *bool | |||
| 27 | + | } | |||
| 28 | + | ||||
| 29 | + | func (r closeTrackReadCloser) Close() error { *r.closed = true; return nil } | |||
| 30 | + | ||||
| 24 | func (p *fakeBootstrapProcess) StdoutPipe() (io.ReadCloser, error) { return p.stdout, nil } | 31 | func (p *fakeBootstrapProcess) StdoutPipe() (io.ReadCloser, error) { return p.stdout, nil } | ||
| 25 | func (p *fakeBootstrapProcess) Start() error { return p.startErr } | 32 | func (p *fakeBootstrapProcess) Start() error { return p.startErr } | ||
| 26 | func (p *fakeBootstrapProcess) Kill() error { p.killed = true; return nil } | 33 | func (p *fakeBootstrapProcess) Kill() error { p.killed = true; return nil } | ||
| @@ -43,22 +50,24 @@ func withListenUDP(t *testing.T, fn func(context.Context) (net.PacketConn, error | |||||
| 43 | func TestRemoteDialerUDPBootstrapFailures(t *testing.T) { | 50 | func TestRemoteDialerUDPBootstrapFailures(t *testing.T) { | ||
| 44 | key := base64.StdEncoding.EncodeToString(make([]byte, pdgram.KeySize)) | 51 | key := base64.StdEncoding.EncodeToString(make([]byte, pdgram.KeySize)) | ||
| 45 | tests := []struct { | 52 | tests := []struct { | ||
| 46 | - | name string | 53 | + | name string |
| 47 | - | stdout string | 54 | + | stdout string |
| 48 | - | start error | 55 | + | start error |
| 49 | - | listen error | 56 | + | listen error |
| 50 | - | want string | 57 | + | want string |
| 51 | - | killed bool | 58 | + | killed bool |
| 52 | - | waited bool | 59 | + | waited bool |
| 60 | + | wantStdoutClosed bool | |||
| 53 | }{ | 61 | }{ | ||
| 54 | - | {name: "start failure", start: errors.New("ssh missing"), want: "vev: remote UDP transport unavailable: start bootstrap: ssh missing"}, | 62 | + | {name: "start failure", start: errors.New("ssh missing"), want: "vev: remote UDP transport unavailable: start bootstrap: ssh missing", wantStdoutClosed: true}, |
| 55 | - | {name: "malformed readiness", stdout: "hello\n", want: "malformed readiness line", killed: true, waited: true}, | 63 | + | {name: "malformed readiness", stdout: "hello\n", want: "malformed readiness line", killed: true, waited: true, wantStdoutClosed: true}, |
| 56 | - | {name: "bootstrap wait failure", stdout: "VEV-UDP 4444 " + key + "\n", want: "wait bootstrap: exit status 255", waited: true}, | 64 | + | {name: "bootstrap wait failure", stdout: "VEV-UDP 4444 " + key + "\n", want: "wait bootstrap: exit status 255", waited: true, wantStdoutClosed: true}, |
| 57 | - | {name: "packet listen failure", stdout: "VEV-UDP 4444 " + key + "\n", listen: errors.New("bind denied"), want: "listen UDP: bind denied", waited: true}, | 65 | + | {name: "packet listen failure", stdout: "VEV-UDP 4444 " + key + "\n", listen: errors.New("bind denied"), want: "listen UDP: bind denied", waited: true, wantStdoutClosed: true}, |
| 58 | } | 66 | } | ||
| 59 | for _, tt := range tests { | 67 | for _, tt := range tests { | ||
| 60 | t.Run(tt.name, func(t *testing.T) { | 68 | t.Run(tt.name, func(t *testing.T) { | ||
| 61 | - | proc := &fakeBootstrapProcess{stdout: io.NopCloser(strings.NewReader(tt.stdout)), startErr: tt.start} | 69 | + | stdoutClosed := false |
| 70 | + | proc := &fakeBootstrapProcess{stdout: closeTrackReadCloser{Reader: strings.NewReader(tt.stdout), closed: &stdoutClosed}, startErr: tt.start} | |||
| 62 | if tt.name == "bootstrap wait failure" { | 71 | if tt.name == "bootstrap wait failure" { | ||
| 63 | proc.waitErr = errors.New("exit status 255") | 72 | proc.waitErr = errors.New("exit status 255") | ||
| 64 | } | 73 | } | ||
| @@ -79,6 +88,9 @@ func TestRemoteDialerUDPBootstrapFailures(t *testing.T) { | |||||
| 79 | if proc.waited != tt.waited { | 88 | if proc.waited != tt.waited { | ||
| 80 | t.Fatalf("waited=%v, want %v", proc.waited, tt.waited) | 89 | t.Fatalf("waited=%v, want %v", proc.waited, tt.waited) | ||
| 81 | } | 90 | } | ||
| 91 | + | if stdoutClosed != tt.wantStdoutClosed { | |||
| 92 | + | t.Fatalf("stdout closed=%v, want %v", stdoutClosed, tt.wantStdoutClosed) | |||
| 93 | + | } | |||
| 82 | }) | 94 | }) | ||
| 83 | } | 95 | } | ||
| 84 | } | 96 | } | ||
M internal/usecase/client/reconnect_toast_test.go +1 -1
| @@ -183,7 +183,7 @@ func TestReconnectSleepWithResizeEventsRedrawsUntilTimerFires(t *testing.T) { | |||
| 183 | 183 | ||
| 184 | 184 | func TestRemoteReconnectToastLifecycleWithWrappedTransportError(t *testing.T) { | |
| 185 | 185 | linkDead := errors.New("remote link dead") | |
| 186 | - | wrappedLinkDead := errors.Join(fmt.Errorf("remote transport receive failed: %w", linkDead)) | |
| 186 | + | wrappedLinkDead := errors.Join(fmt.Errorf("remote transport receive failed: %w", linkDead), errors.New("read side closed")) | |
| 187 | 187 | oldSleep := reconnectSleep | |
| 188 | 188 | oldSleepWithResize := reconnectSleepWithResize | |
| 189 | 189 | reconnectSleep = func(context.Context, ports.Clock, time.Duration) bool { return true } | |
| @@ -183,7 +183,7 @@ func TestReconnectSleepWithResizeEventsRedrawsUntilTimerFires(t *testing.T) { | |||||
| 183 | 183 | ||||
| 184 | func TestRemoteReconnectToastLifecycleWithWrappedTransportError(t *testing.T) { | 184 | func TestRemoteReconnectToastLifecycleWithWrappedTransportError(t *testing.T) { | ||
| 185 | linkDead := errors.New("remote link dead") | 185 | linkDead := errors.New("remote link dead") | ||
| 186 | - | wrappedLinkDead := errors.Join(fmt.Errorf("remote transport receive failed: %w", linkDead)) | 186 | + | wrappedLinkDead := errors.Join(fmt.Errorf("remote transport receive failed: %w", linkDead), errors.New("read side closed")) |
| 187 | oldSleep := reconnectSleep | 187 | oldSleep := reconnectSleep | ||
| 188 | oldSleepWithResize := reconnectSleepWithResize | 188 | oldSleepWithResize := reconnectSleepWithResize | ||
| 189 | reconnectSleep = func(context.Context, ports.Clock, time.Duration) bool { return true } | 189 | reconnectSleep = func(context.Context, ports.Clock, time.Duration) bool { return true } | ||