containerd / containerd/ttrpc

Handle forced connection closed by remote host error on Windows in release/1.1 branch

Open
#164 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
664
Forks
86
PR merge metrics
No merged PRs in 30d

Description

### Problem

When enabling CI on Windows for release/1.1 branch (#163), an edge case was found via unit tests when running on the latest Windows platform with Go 1.20+ toolchain.

```
=== RUN TestOversizeCall
--- PASS: TestOversizeCall (0.03s)
=== RUN TestClientEOF
server_test.go:365: expected to have a cause of ErrClosed, got read unix @->TestClientEOF: wsarecv: An existing connection was forcibly closed by the remote host.
--- FAIL: TestClientEOF (0.00s)
=== RUN TestServerRequestTimeout
--- PASS: TestServerRequestTimeout (0.01s)
=== RUN TestServerConnectionsLeak
--- PASS: TestServerConnectionsLeak (0.10s)
=== RUN Test_MethodFullNameGeneration
--- PASS: Test_MethodFullNameGeneration (0.00s)
FAIL
github.com/containerd/ttrpc coverage: 76.3% of statements
FAIL github.com/containerd/ttrpc 1.696s
? github.com/containerd/ttrpc/cmd/protoc-gen-gogottrpc [no test files]
? github.com/containerd/ttrpc/example [no test files]
? github.com/containerd/ttrpc/example/cmd [no test files]
? github.com/containerd/ttrpc/plugin [no test files]
FAIL
```

The root cause is client not able to filter the connection closed by server error on Windows platform with the updated toolchain.
Ref: https://github.com/containerd/ttrpc/blob/20c493e60193d417ec0c6e795a4dcb8f25221f7d/client.go#L385

The impact is the client error does not get set to ErrClosed as expected.
Ref: https://github.com/containerd/ttrpc/blob/20c493e60193d417ec0c6e795a4dcb8f25221f7d/client.go#L38

### Recommended Solution
The recommended solution would be to filter the error based on platform error using golang.org/x/sys package.

The solution should enable testing on Windows platform and existing TestClientEOF test should be sufficient to determine if issue is resolved.

### Alternative solutions considered

#### Filter `io.ErrClosedPIpe` (Not viable)
This solution was to filter error closed on `io.ErrClosedPipe`.
Ref: https://github.com/containerd/ttrpc/blame/4a2816be9b4843f2cf40f1e533e7830d23f4489b/client.go#L450

Opened #165 and found this approach did not resolve the issue.

### Filter based on error string
This solution would filter error closed based on the error string.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.