Skip to content

Fast-fail on connection errors#567

Merged
dragonsinth merged 1 commit into
fullstorydev:masterfrom
bcleenders:surface-post-dial-conn-errors
Jun 13, 2026
Merged

Fast-fail on connection errors#567
dragonsinth merged 1 commit into
fullstorydev:masterfrom
bcleenders:surface-post-dial-conn-errors

Conversation

@bcleenders

@bcleenders bcleenders commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Avoid hanging on connection errors; fail fast by propagating connection errors (not just for TLS).

This also speeds up tests a lot (from >20s to about 3s), since the tests included some connection timeouts.

This expands on #564 ; it does the same for plaintext connections.

Fixes #387

Demo

Start server that expects TLS (to mimick the issue linked above):

$ go run ./internal/testing/cmd/testserver \
    -cert internal/testing/tls/server.crt \
    -key internal/testing/tls/server.key \
    -p 50051

Without fix:

$ grpcurl -plaintext localhost:50051 list
Failed to dial target host "localhost:50051": context deadline exceeded

With fix:

$ ./grpcurl -plaintext localhost:50051 list
Failed to dial target host "localhost:50051": read tcp 127.0.0.1:64984->127.0.0.1:50051: read: connection reset by peer

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 fullstorydev#564 ; it
does the same for plaintext connections.

Fixes fullstorydev#387

@dragonsinth dragonsinth left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice improvement!

@dragonsinth
dragonsinth merged commit 4ea1554 into fullstorydev:master Jun 13, 2026
3 checks passed
@bcleenders
bcleenders deleted the surface-post-dial-conn-errors branch June 15, 2026 13:42
@dragonsinth

Copy link
Copy Markdown
Member

FYI: we've been getting test flakes recently--

--- FAIL: TestBrokenTLS_RequireClientCertButNonePresented (0.02s)
    tls_settings_test.go:332: expecting a TLS certificate error, got: read tcp 127.0.0.1:49228->127.0.0.1:42915: use of closed network connection

It's either this PR, or #564 - unsure. Would you mind taking a look @bcleenders ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

grpcurl fails with "context deadline exceeded" after 10s if using plaintext when server expects TLS

2 participants