[Bug]: Reaper.connect swallows a failed handshake, leaving the process silently unregistered with Ryuk #3827

Summary

Reaper.connect treats a failed Ryuk handshake as success. The error is logged and discarded, and a live termination channel is returned anyway, so the caller proceeds believing it registered with Ryuk when it never did. Ryuk then undercounts its clients, and can prune containers that are still in use.

This is the fail-open that made #3743 destructive rather than noisy. That issue's fix (#3761, composing ForLog("Started") into fromContainer's wait) removed the trigger I could reproduce, and I've verified it works. But the fail-open behind it is still on main, so any other cause of a failed handshake — Ryuk restarting, a connection reset under load, a slow daemon — silently reproduces the same class of failure.

https://github.com/testcontainers/testcontainers-go/blob/main/reaper.go

terminationSignal := make(chan bool)
go func() {
    defer conn.Close()
    if err := r.handshake(conn); err != nil {
        log.Printf("Reaper handshake failed: %s", err)   // <- logged, then dropped
    }
    <-terminationSignal
}()
return terminationSignal, nil                            // <- reported as success

Why it matters

The handshake is what registers a client and installs the label filter. A process whose handshake fails is invisible to Ryuk while its containers still carry the shared session label. When the last successfully registered process exits, Ryuk sees clients=0, waits RYUK_RECONNECTION_TIMEOUT, and prunes the whole session — including containers belonging to processes that are still running.

Because the session ID is derived from the parent PID, every package binary in one go test ./... shares one Ryuk, so this is cross-process: one package's exit can destroy another package's database mid-test.

What I observed (on v0.43.0, before #3761)

Ryuk's own log, with two packages running and two containers up:

adding filter ... sessionId=c3be0946...   <- only ONE filter, for two packages
client disconnected  clients=0
prune check clients=0
removed containers=2                      <- one of them still in use

The victim's pool died mid-test (conn closed), then cleanup panicked in Restore with port "5432/tcp" not found because the container was gone. go test ./... failed 4 runs in 5 on a 20-core machine; it passed at -p 2, where packages are staggered enough that a new one always reconnects inside the 10s window.

Confirming #3761 fixes the trigger: same suite, same machine, dependency bumped to c92ea61 — 4/4 passes and zero Reaper handshake failed lines, against ~9 per run before.

Suggested change

Make a failed handshake a failed connect, so retryLocked's existing retry/backoff can handle it, rather than silently continuing unregistered:

conn, err := d.DialContext(ctx, "tcp", r.Endpoint)
if err != nil { ... }
if err := r.handshake(conn); err != nil {
    conn.Close()
    return nil, fmt.Errorf("handshake reaper %s: %w", r.Endpoint, err)
}

That needs the handshake to move ahead of the goroutine that waits on terminationSignal, which looks straightforward since handshake is already a synchronous write/read pair.

If propagating is considered too strict, even retrying the handshake before giving up would be a large improvement over the current fail-open — the failure mode it hides is silent data-destroying container removal, which is much worse than a loud connect error.

I'm happy to open a PR if the direction looks right.

Testcontainers version

v0.43.0, and main as of c92ea61

Host OS / arch

Linux x86_64 (20 cores)

Go version

go1.25