Skip to content

fix: name selected Docker connection in DMR transport errors - #4491

Merged
dgageot merged 1 commit into
docker:mainfrom
dgageot:fix/dmr-tunnel-error-context
Oct 1, 2026
Merged

dgageot merged 1 commit into
docker:mainfrom
dgageot:fix/dmr-tunnel-error-context

Conversation

@dgageot

@dgageot dgageot commented Oct 1, 2026

Copy link
Copy Markdown
Member

Since #4481, DMR requests over an explicitly selected Docker connection dial through a subprocess started via commandconn.New for docker system dial-stdio. That subprocess can exit quickly, and the resulting net.Conn Read/Write calls then surface as a bare closed or broken pipe error with no indication of which Docker connection (--host, --context, etc.) was in use. TestDMRDockerConnectionErrorNamesEngine catches this on Linux but is flaky because of the race between the subprocess exiting and the first read, which also made it fail intermittently under #4490 even though that PR never touched this code path.

This adds a dockerTransport that wraps *http.Transport and, on a failed RoundTrip, returns a *net.OpError naming the selected connection's args while preserving errors.Is matching, Timeout/Temporary, and CloseIdleConnections. The wrapper is only used when a Docker connection was explicitly selected; the default unix-socket path is untouched. resolve_test.go adds a deterministic table of dial errors, closed/broken pipes, EOF, and context cancellation/deadline so the error-naming behavior no longer depends on subprocess timing.

Validated with task build, task test, and task lint, plus targeted repeats of the new and existing DMR tests (go test -race ./pkg/model/provider/dmr/dmrmodels -count=20, and Linux-only repeats of TestDMRDockerConnectionErrorNamesEngine and the new deterministic test) to confirm no remaining flakiness.

The Docker system dial-stdio subprocess started via commandconn.New can exit quickly, surfacing errors on net.Conn Read/Write as a plain closed or broken pipe with no mention of which Docker connection was selected.

Wrap the HTTP transport used for an explicitly selected Docker connection so a failed RoundTrip returns a *net.OpError naming the selected connection args, while preserving errors.Is, Timeout/Temporary and CloseIdleConnections behavior. The default, unselected transport path is untouched.
@dgageot
dgageot requested a review from a team as a code owner October 1, 2026 07:51
@dgageot
dgageot added this pull request to the merge queue Oct 1, 2026
@aheritier aheritier added area/providers/docker-model-runner Docker Model Runner (DMR) local inference kind/fix PR fixes a bug (maps to fix:). Use on PRs only. labels Oct 1, 2026
Merged via the queue into docker:main with commit 255fdb1 Oct 1, 2026
16 checks passed
@dgageot
dgageot deleted the fix/dmr-tunnel-error-context branch October 1, 2026 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/providers/docker-model-runner Docker Model Runner (DMR) local inference kind/fix PR fixes a bug (maps to fix:). Use on PRs only.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants