feat: Auto-reconnect to Docker daemon when connection drops

[amir20]

Summary

Implements automatic reconnection to Docker daemons when the connection is lost (e.g., during daemon restart). Previously, a broken connection would freeze the UI on a stale container list. Now the tool detects connection loss, shows a "reconnecting" banner, and automatically re-synchronizes once the daemon is reachable again.

Key Changes

• Connection recovery loop: container_manager() now wraps the fetch/monitor cycle in a loop that detects connection failures and retries with exponential backoff (1s → 5s max)

• Return value from fetch_initial_containers(): Now returns bool to indicate success/failure, allowing the caller to detect when the daemon is unreachable

• New wait_until_reachable() method: Pings the daemon with capped exponential backoff until it responds, preventing reconnect loop spin

• Event stream error handling: Changed from silently sleeping on error to returning immediately, since Bollard's event stream doesn't recover from broken connections

• New app events:

  • HostDisconnected(HostId) - Emitted when connection is lost, triggers "reconnecting" banner
  • HostReconnected(HostId) - Emitted when daemon becomes reachable again

• UI reconnecting banner: New render_status_notifications() function displays persistent "⟳ host: connection lost, reconnecting…" banners in the top right for disconnected hosts

• Container re-synchronization: InitialContainerList is now re-sent on reconnect as the authoritative list, so stale containers are dropped and new ones are picked up. Empty lists are sent to clear containers when a host has none.

• Selection preservation: When re-synchronizing, the UI preserves user selection if possible, or clamps it to valid range

• Per-container stats cleanup: Stats streaming tasks are aborted on disconnect so reconnect starts from a clean slate

Implementation Details

• Disconnected hosts are tracked in a BTreeSet for stable rendering order
• Connection errors are separate from disconnection state (errors expire, disconnection persists until reconnect)
• The reconnect loop checks if the event sender is closed (app quitting) to avoid unnecessary retries
• Logging added at warn/info levels for connection loss and recovery events
• Comprehensive tests added for re-synchronization behavior and UI rendering

Fixes #333

[github-actions]

Docker Image Built Successfully

docker pull ghcr.io/amir20/dtop:pr-334

[claude]

Review

Solid fix for #333 — the loop/backoff/resync structure in container_manager is clean and the AppState-side resync logic (drop-then-reinsert per host, index clamping) is well covered by the new tests. A few things worth a look before merge:

1. Any event-stream error now triggers a full disconnect/reconnect cycle (src/docker/connection.rs, monitor_docker_events)
Previously an Err from events_stream.next() just slept 1s and kept reading; now it unconditionally returns, which the caller treats as a full connection loss: stats tasks aborted, "reconnecting" banner shown, and on recovery the whole container list is re-fetched (with a sequential inspect_container call per container for restart_count). The comment asserts "Bollard's event stream does not recover from a broken connection" — if that's true for every error variant the stream can yield (including any transient/single-event decode errors, not just transport-level failures), this is correct and actually an improvement (removes a pointless sleep-then-silently-die path). But if bollard can ever emit a recoverable per-event error without killing the underlying connection, this will now cause visible banner flicker + a full re-list/re-inspect sweep for something that used to be a no-op retry. Worth double-checking against bollard's actual error semantics, or narrowing the fatal branch to transport-level errors specifically.

2. Selection "preservation" is index-based, not identity-based (container_events.rs::handle_initial_container_list)
On resync, the previously-selected index is kept (via clamp_selection), not the previously-selected container. Since resync also resets stats to defaults for the reconnected host's containers, a resort (e.g. sort-by-CPU/Memory) could reshuffle rows so the same index now points at a different container. This matches the app's existing general sort behavior (selection has always been index-based elsewhere), so it's not a regression per se, but the PR description's "preserves user selection if possible" is a bit stronger than what the code actually guarantees — worth calling out in case a user opens the action menu right after a reconnect and hits the wrong container.

3. Test coverage gap on the actual reconnect state machine
The new tests are thorough for AppState event handling and the banner rendering, but container_manager's loop, wait_until_reachable's backoff, and fetch_initial_containers's new bool return are untested — understandably hard without a mock Docker daemon, but it means the core new logic of this PR has no automated safety net. Not a blocker, just flagging the gap.

Nothing security-sensitive stood out (no new untrusted input handling), and the CLAUDE.md doc updates accurately reflect the new architecture.

[amir20]

Thanks — points 2 and 3 were worth acting on and are addressed in a375b5c. Point 1 I dug into and it turns out not to be a live concern; details below.

1. Event-stream errors — no recoverable variant exists

I traced the stream, and there is no error the events stream can yield that leaves it usable, so the fatal branch can't be narrowed.

Docker::events → process_into_stream → decode_into_stream builds a FramedRead<StreamReader<Incoming>, JsonLineDecoder>. tokio-util's FramedRead routes both decoder errors (decode returns Err — the malformed-JSON case) and reader errors (transport failure) through the same has_errored = true transition, and the state-machine diagram in framed_impl.rs shows both edges landing in Errored. The next poll then hits:

// Return `None` if we have encountered an error from the underlying decoder
// See: https://github.com/tokio-rs/tokio/issues/3976
if state.has_errored { ... return Poll::Ready(None); }

So the stream yields Some(Err(_)) once and then terminates on the very next poll, whatever the error was. try_flatten above it then sees the inner stream end and finishes the outer stream too.

That has a consequence worth noting: the old code was already dead in this path. Err → sleep 1s → continue → next() immediately returns None → loop exits → container_manager returns and the host stops being monitored forever. The 1s sleep wasn't a retry, it was a pause before dying — which is exactly the reported bug. So return isn't a new escalation; it's the same exit minus the pointless sleep, with the caller now reconnecting instead of giving up. And since the stream is dead either way, re-subscribing is mandatory rather than a choice, which also makes the re-list correct: events can be missed while re-subscribing, so a resync is the only way back to accurate state.

I've kept the comment but made the reasoning checkable rather than asserted.

2. Index-based selection — fixed, and it was sharper than "not a regression"

You were right to flag it, and the resync case is worse than the general sort case. A resync resets the host's stats to ContainerStats::default(), so under a CPU/Memory sort every row compares equal — total_cmp returns Equal, the tiebreak is host_id which is identical within a host, and the input order comes from HashMap iteration. The post-reconnect order is effectively arbitrary, so an index-based cursor doesn't just drift, it lands somewhere unrelated. With Remove in the action menu one Enter away, that's worth closing.

handle_initial_container_list now captures the selected ContainerKey before the swap and restores by identity, falling back to the old index-clamp only when that container is genuinely gone. Two tests cover it, and the first fails if the identity lookup is removed.

I deliberately scoped this to the resync path rather than making selection identity-based everywhere — that's a broader behavioral change than this PR should carry, and elsewhere the list doesn't reshuffle underneath the user the way it does here. Happy to do it separately if you want it.

3. Test coverage — added

Fair, and it was the gap that mattered most. src/docker/reconnect_tests.rs runs a real Bollard client against a stand-in daemon on a unix socket that can be killed and rebound at the same path:

• container_manager_reconnects_after_daemon_restart — daemon serves one container set, is killed mid-stream, then comes back serving a different set. Asserts HostDisconnected → HostReconnected → a re-synced list matching the new set, so a resync is distinguishable from replayed pre-outage state. This also pins down the thing I'd otherwise only assumed: that the reused Docker client recovers once the socket reappears.
• container_manager_reports_disconnect_when_host_never_answers — unreachable host keeps retrying instead of exiting.
• container_manager_stops_when_event_channel_closes — the loop terminates when the UI is gone rather than retrying forever.

Both reconnect tests fail if the loop is reverted to the old straight-line form (verified). The backoff policy is extracted into next_reconnect_delay and unit-tested for the doubling and the cap. Runtime is ~1s, no flakiness over repeated runs; socket paths are process-scoped since the module tree compiles into more than one test binary.

One thing the fake daemon deliberately does not cover: stats streams restarting after a reconnect. That needs the /stats endpoint and running containers, which adds a fair bit of fixture for a path that's just start_container_monitoring being called again. I verified it manually instead — against a real dockerd, SIGTERM'd and restarted, stats events resumed after the resync.

cargo fmt, clippy --all-targets --all-features -D warnings, and 144 tests all pass.

---

Generated by Claude Code

[claude]

Review

Solid fix for #333 — the reconnect state machine is well designed (bounded backoff, identity-based selection tracking across a resync, tests that drive a real socket-level daemon restart rather than mocking). A few notes:

Correctness

• src/core/app_state/container_events.rs:17-21 — selecting by container identity instead of row index across a resync is the right call, since a resync zeroes stats and can reorder a stats-sorted list. Good catch, and it's covered by test_resync_keeps_selection_on_same_container.
• Event ordering nit (non-blocking): in container_manager (src/docker/connection.rs:593-597), HostReconnected is sent before the next loop iteration calls fetch_initial_containers, so the "reconnecting" banner can disappear a beat before the container list is actually re-synced. Stale rows stay visible in the meantime (not blank), so this is cosmetic, but flipping the order (or sending HostReconnected only after a successful resync) would tighten it up.
• Log view during a disconnect isn't addressed: docker/logs.rs::stream_container_logs just breaks silently on a stream error and isn't in this PR's abort list (only the stats tasks in active_containers get aborted on disconnect). If a user is viewing logs for a container when the host drops, the log stream dies with no indicator and won't resume after reconnect until they back out and re-enter. Probably fine to leave as a follow-up given the issue was scoped to the container list, but worth a tracking issue.

Design

• wait_until_reachable always sleeping before the first ping attempt is a nice touch — it prevents a daemon that repeatedly drops the event stream immediately from spinning the reconnect loop hot.
• handle_host_disconnected deliberately leaves stale containers in place rather than clearing them — reasonable UX call (avoids a blank screen during a transient outage) and it's documented inline.

Tests

• src/docker/reconnect_tests.rs is a genuinely strong addition — a real stand-in daemon over a unix socket that can be killed/rebound in place exercises the actual Bollard client behavior (socket drop, ping-based recovery, resync) rather than testing the state machine in isolation. The container_manager_stops_when_event_channel_closes test closing the channel before spawning also correctly avoids a startup race.
• Both container_manager_reconnects_after_daemon_restart and the "never answers" test rely on real wall-clock sleeps (1-5s backoff) rather than paused tokio time, so they'll add several seconds to the suite; not a problem at this scale but could be a target for start_paused = true if the backoff constants grow later.

Minor

• render_status_notifications (src/ui/render.rs) sizes the banner with text.chars().count(), which is fine for the current fixed-format strings but would undercount for wide-display host names if that ever changes — not an issue today since host_id is effectively ASCII (derived from URLs).

Overall this looks correct and well-tested; happy to see this merged once CI is green.

[claude]

Review

Solid fix for a real problem (frozen UI after a Docker daemon restart), and the reconnect state machine is well-tested — the fake-daemon integration tests in reconnect_tests.rs actually exercise a real Bollard client surviving a dropped/rebound unix socket, which is much stronger than mocking. Nice attention to detail in the code comments (e.g. citing tokio-rs/tokio#3976 for why the event stream error is terminal).

A few things worth a look:

1. Selection fallback can still land on the wrong container (container_events.rs:38-47)
When the previously-selected container survives a resync, selection now correctly follows it by identity — that's the main fix and it works. But when the selected container is gone, the fallback calls clamp_selection(), which only clamps an out-of-range index; if the old index is still in bounds it leaves the cursor on that row untouched. Since fetch_initial_containers always resets stats to ContainerStats::default(), any stats-based sort (CPU/Memory/Net/Disk) ties every container at 0 on every resync, and the tiebreaker is only host_id (sorting.rs:186) — so the actual order among same-host containers falls back to HashMap iteration order, which isn't guaranteed stable across the fresh HashMap built by retain+insert in this same handler. In other words, it's plausible for the cursor to silently end up on an unrelated container after the one it was on disappears — a narrower version of exactly the "next Enter opens the action menu on a container the user never picked" hazard described in the commit message as the motivation for this fix. Might be worth explicitly resetting to row 0 (or nearest-by-original-position) in that fallback branch rather than reusing the stale numeric index.

2. Reconnect banner text isn't truncated (render.rs:184-207)
The new disconnected-host banner caps width at 80 but never shortens text itself, unlike the connection-error banner right below it which truncates error_msg to 80 chars with ... before computing width. A long host id (e.g. a long SSH connection string) will just get silently clipped by the Paragraph instead of gracefully truncated — minor inconsistency between the two nearly-identical code paths in the same function.

3. wait_until_reachable doesn't check for shutdown (connection.rs:429-443)
The outer container_manager loop checks tx.is_closed() before and after waiting, so it stops retrying once the app quits. But wait_until_reachable's own loop only exits on a successful ping — if a host is unreachable when the app quits, it keeps pinging (up to every 5s) until it happens to succeed, rather than bailing out promptly like the surrounding loop does. Since the whole tokio runtime is dropped on process exit this is likely harmless in practice, but it's a small inconsistency with the stated goal ("checks if the event sender is closed... to avoid unnecessary retries").

Nothing here is a blocker — #1 is the one worth double-checking since it touches the exact hazard this PR set out to close; #2 and #3 are minor. Test coverage looks good otherwise (resync semantics, selection-by-identity, empty-list clearing, and a UI snapshot for the banner are all covered).