Avoid hanging on connection errors; fail fast by propagating connection
errors.
This also speeds up tests a lot (from >20s to about 3s), since the tests
included some connection timeouts.
This expands on https://github.com/fullstorydev/grpcurl/pull/564 ; it
does the same for plaintext connections.
Fixes#387
* Add support for TLS 1.3
This PR allows TLS 1.3, by removing the MaxVersion in the client config.
This would silently swallow errors, so e.g. a client without cert
dialing a server that requires client certs would lead to an error which
gets ignored, leading to retries until timeout.
In this PR, we wrap the connection and if an error occurs we send it to
the existing `result` channel.
I think this matches @jhump's comment in https://github.com/fullstorydev/grpcurl/issues/387#issuecomment-1517098394
**Testing**
```console
# Start the test server (in another tab)
go run ./internal/testing/cmd/testserver \
-cert internal/testing/tls/server.crt \
-key internal/testing/tls/server.key \
-cacert internal/testing/tls/ca.crt \
-requirecert -p 9999
# Old behavior
$ grpcurl -cacert internal/testing/tls/ca.crt \
localhost:9999 list
Failed to dial target host "localhost:9999": context deadline exceeded
# New behavior
$ go run ./cmd/grpcurl -cacert internal/testing/tls/ca.crt \
localhost:9999 list
Failed to dial target host "localhost:9999": remote error: tls: certificate required
exit status 1
```
The old behavior is to hang until we hit the deadline. The new behavior
is to return immediately with an error.
Fixes#563
* Implement review comments
Removing unnecessary sync.Once, as the channel will already drop repeated calls.
The error message for client certificate failures changed in Go 1.25.
Update tests to check for both the old ("bad certificate") and new
("handshake failure") error strings to support multiple Go versions.
Signed-off-by: Mikel Olasagasti Uranga <mikel@olasagasti.info>
* Run tests on Go 1.21
For the most part, there are no breaking changes.
However, the expired certificate is now showing "expired certificate"
although previously it showed a simpler "bad certificate" which was
hard-coded into the TLS settings test scenario.
* Simplify condition for certificate error
Instead of two `expired certificate` and `bad certificate` comparisons, we can just check for `certificate` in error output. This satisfies us when checking there is something wrong with the certificate.
Co-authored-by: Scott Blum <dragonsinth@gmail.com>
---------
Co-authored-by: Scott Blum <dragonsinth@gmail.com>
