Staleness probe reads an unreachable Docker daemon as "no container" #56

Open
opened 2026-09-11 13:25:45 +00:00 by jknapp · 0 comments
Owner

get_container_staleness reads an unreachable Docker daemon as "no container", so a transient daemon fault produces a message that is confidently wrong:

This project has no container or snapshot image yet, so there is nothing to compare against the base image.

Found while fixing #55, where stopped containers were reported the same way. That PR narrowed the message to the case it is actually true of, but left this one: an Err from the daemon still lands on the same arm as a genuine absence.

Where

Four swallowed Results, all inside get_container_staleness in app/src-tauri/src/commands/migration_commands.rs:

Line Call
217 mig::image_id(&base_image).await.unwrap_or(None)
231 docker::find_existing_container(&project).await.unwrap_or(None)
252 docker::is_container_running(id).await.unwrap_or(false)
255 docker::image_exists(&snapshot_image).await.unwrap_or(false)

Why it is small

No signature changes, and no ripple to other callers. All four already return Result<_, String>, and all four already distinguish absent from unreachable: find_existing_container and image_exists return Ok with an empty filtered list for "not found" and Err only on a transport failure, and image_id maps 404 to Ok(None). The information exists and is being discarded at the call site.

Suggested shape: collect the probe inputs through one small pure function that takes the Results and returns either the inputs or the first daemon error. That gives a unit-testable seam — the approach pick_probe_source and stopped_probe_policy already take in this file — instead of untestable I/O.

No frontend work. probeUnavailable = !staleness.known && !!staleness.probe_error in ContainerMigrationBanner.tsx already routes this to "Some checks did not complete: ...". It only needs a message that says the daemon was unreachable.

Estimate: about an hour, roughly a 60-line diff including tests.

Decision to make when picking this up

On a daemon error, set probe_error rather than returning Err. The hook's catch sets staleness to null, which makes the banner disappear entirely — hiding the fault instead of reporting it.

Why it is not urgent

The frontend only calls this after Docker is confirmed available, so the realistic trigger is a transient fault (daemon restarting, socket permissions changing, daemon briefly wedged) rather than a cold-down daemon. The payoff is a rare misleading message becoming an accurate one.

Out of scope

Similar unwrap_or swallows further down the same file (the migration paths, around lines 515, 571, 674, 803). Some are legitimately "unknown is fine, probe instead" — they deserve their own audit, not a blanket change.

Generated with Claude Code

https://claude.ai/code/session_019RSaoDLovVV2wmH4H8VVxz

`get_container_staleness` reads an unreachable Docker daemon as "no container", so a transient daemon fault produces a message that is confidently wrong: > This project has no container or snapshot image yet, so there is nothing to compare against the base image. Found while fixing #55, where stopped containers were reported the same way. That PR narrowed the message to the case it is actually true of, but left this one: an `Err` from the daemon still lands on the same arm as a genuine absence. ## Where Four swallowed `Result`s, all inside `get_container_staleness` in `app/src-tauri/src/commands/migration_commands.rs`: | Line | Call | |------|------| | 217 | `mig::image_id(&base_image).await.unwrap_or(None)` | | 231 | `docker::find_existing_container(&project).await.unwrap_or(None)` | | 252 | `docker::is_container_running(id).await.unwrap_or(false)` | | 255 | `docker::image_exists(&snapshot_image).await.unwrap_or(false)` | ## Why it is small **No signature changes, and no ripple to other callers.** All four already return `Result<_, String>`, and all four already distinguish absent from unreachable: `find_existing_container` and `image_exists` return `Ok` with an empty filtered list for "not found" and `Err` only on a transport failure, and `image_id` maps 404 to `Ok(None)`. The information exists and is being discarded at the call site. Suggested shape: collect the probe inputs through one small **pure** function that takes the `Result`s and returns either the inputs or the first daemon error. That gives a unit-testable seam — the approach `pick_probe_source` and `stopped_probe_policy` already take in this file — instead of untestable I/O. **No frontend work.** `probeUnavailable = !staleness.known && !!staleness.probe_error` in `ContainerMigrationBanner.tsx` already routes this to "Some checks did not complete: ...". It only needs a message that says the daemon was unreachable. Estimate: about an hour, roughly a 60-line diff including tests. ## Decision to make when picking this up On a daemon error, set `probe_error` rather than returning `Err`. The hook's `catch` sets `staleness` to `null`, which makes the banner disappear entirely — hiding the fault instead of reporting it. ## Why it is not urgent The frontend only calls this after Docker is confirmed available, so the realistic trigger is a *transient* fault (daemon restarting, socket permissions changing, daemon briefly wedged) rather than a cold-down daemon. The payoff is a rare misleading message becoming an accurate one. ## Out of scope Similar `unwrap_or` swallows further down the same file (the migration paths, around lines 515, 571, 674, 803). Some are legitimately "unknown is fine, probe instead" — they deserve their own audit, not a blanket change. Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_019RSaoDLovVV2wmH4H8VVxz
Sign in to join this conversation.
No labels
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: CyberCoveLLC/Triple-C#56