synced/coder
1
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2025-07-18 14:17:22 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(cli): fix flakes related to context cancellation when establishing pg connections (#18246)

Browse Source 
Since https://github.com/coder/coder/pull/18195 was merged, we started
running CLI tests with postgres instead of just dbmem. This surfaced
errors related to context cancellation while establishing postgres
connections.

This PR should fix https://github.com/coder/internal/issues/672. Related
to https://github.com/coder/coder/issues/15109.
This commit is contained in:
Hugo Dutka
2025-06-05 15:54:13 +02:00
committed by GitHub
parent b5fd3dd855
commit 623dcd97dc
3 changed files with 17 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
cli/clitest/clitest.go
View File

@ -168,6 +168,12 @@ func StartWithAssert(t *testing.T, inv *serpent.Invocation, assertCallback func(
switch { switch {
case errors.Is(err, context.Canceled): case errors.Is(err, context.Canceled):
return return
case err != nil && strings.Contains(err.Error(), "driver: bad connection"):
// When we cancel the context on a query that's being executed within
// a transaction, sometimes, instead of a context.Canceled error we get
// a "driver: bad connection" error.
// https://github.com/lib/pq/issues/1137
return
default: default:
assert.NoError(t, err) assert.NoError(t, err)
} }

5
cli/server.go
View File

@ -2359,6 +2359,10 @@ func ConnectToPostgres(ctx context.Context, logger slog.Logger, driver string, d
if err != nil { if err != nil {
return nil, xerrors.Errorf("get postgres version: %w", err) return nil, xerrors.Errorf("get postgres version: %w", err)
} }
defer version.Close()
if version.Err() != nil {
return nil, xerrors.Errorf("version select: %w", version.Err())
}
if !version.Next() { if !version.Next() {
return nil, xerrors.Errorf("no rows returned for version select") return nil, xerrors.Errorf("no rows returned for version select")
} }
@ -2367,7 +2371,6 @@ func ConnectToPostgres(ctx context.Context, logger slog.Logger, driver string, d
if err != nil { if err != nil {
return nil, xerrors.Errorf("scan version: %w", err) return nil, xerrors.Errorf("scan version: %w", err)
} }
_ = version.Close()
if versionNum < 130000 { if versionNum < 130000 {
return nil, xerrors.Errorf("PostgreSQL version must be v13.0.0 or higher! Got: %d", versionNum) return nil, xerrors.Errorf("PostgreSQL version must be v13.0.0 or higher! Got: %d", versionNum)

13
coderd/database/pubsub/pubsub.go
View File

@ -552,16 +552,15 @@ func (p *PGPubsub) startListener(ctx context.Context, connectURL string) error {
sentErrCh = true sentErrCh = true
}), }),
} }
select { // We don't respect context cancellation here. There's a bug in the pq library
case err = <-errCh: // where if you close the listener before or while the connection is being
if err != nil { // established, the connection will be established anyway, and will not be
// closed.
// https://github.com/lib/pq/issues/1192
if err := <-errCh; err != nil {
_ = p.pgListener.Close() _ = p.pgListener.Close()
return xerrors.Errorf("create pq listener: %w", err) return xerrors.Errorf("create pq listener: %w", err)
} }
case <-ctx.Done():
_ = p.pgListener.Close()
return ctx.Err()
}
return nil return nil
} }