client/internal: fix Streams.Wait error handling, fix RSFilterReader error handling by thaJeztah · Pull Request #52305 · moby/moby

thaJeztah · 2026-04-04T14:46:37Z

relates to:

driver/docker-container: remove uses of jsonmessage docker/buildx#3777 (comment)
docker-container: check error from response body when pulling image docker/buildx#2988
Buildx with new/unstarted docker-container driver and containerd can't use just pulled image #49453
fix content-type declared by /events API #50953 (comment)

go.mod: add back replace rules

client/internal: Stream.Wait: fix missing error-handling

The intent of the Wait method is to allow consumers to handle a stream
synchronously; block until the push/pull/build completes (or is cancelled)
but report errors happening during the stream (e.g. failures to pull the
image).

Wait previously ignored errors reported via jsonstream.Message.Error,
causing push/pull failures to be treated as success. Update it to return
these in-band errors while preserving existing handling for decode,
transport, and context cancellation errors.

client/internal: Stream.Wait: return errdefs errors

client/internal: Stream.JSONMessages: move reader inside the func

Just slightly cleaner on scope.

client/internal: Stream.JSONMessages: improve error handling

explicitly return zero-values on error to prevent any potential "garbage"
decoded message from being returned.
do not return decoding errors if the context was cancelled; decoding may
fail due to the context being cancelled (and reader closed), which could
produce unexpected errors.
consolidate all error-handling within a single err != nil block.

client/internal: RSFilterReader: un-export, and remove unused buffer

This buffer was added as part of the initial implementation, but during
review it was decided it's not needed; #50953 (comment)

Remove the buffer as it's not used, and un-export the RSFilterReader
implementation.

client/internal: improve test-coverage for rsFilterReader

client/internal: fix RSFilterReader returning (0, nil) after filtering

RSFilterReader could consume input consisting only of RS bytes and return
(0, nil), violating io.Reader expectations. This can cause callers to loop
indefinitely waiting for progress.

For example, a caller like:

for {
    n, err := r.Read(buf)
    if err != nil {
        return err
    }
    if n == 0 {
        continue
    }
    process(buf[:n])
}

would loop forever if Read returns (0, nil) after consuming input.

Fix by continuing to read until either filtered data is available or an
error (including EOF) is returned.

client/internal: touch-up godoc

Also change NewJSONStreamDecoder to use use types.MediaType as argument. The
MediaType type-alias that was added in 0e7c817
to help discoverability of accepted media/content-types.

- Human readable description for the release notes

client: fix `ImagePullResponse.Wait`, `ImagePushResponse.Wait` not returning an error if pull/push errors happend during the pull operation.

- A picture of a cute animal (not mandatory but encouraged)

thaJeztah · 2026-04-04T14:50:02Z

 			// push/pull failures.
-			return jm.Error
+			return httpErrorFromStatusCode(jm.Error, jm.Error.Code)


We need to do the same in jsonmessage.Display

moby/client/pkg/jsonmessage/jsonmessage.go

Lines 132 to 135 in 9aa5ac3

func Display(jm jsonstream.Message, out io.Writer, isTerminal bool, width uint16) error {

if jm.Error != nil {

return jm.Error

}

Perhaps ideally, we would make this functionality part of jsonstream.Error itself, but that will add containerd/errdefs as dependency to the API.

Copilot

Pull request overview

This PR improves Docker client stream handling so synchronous consumers (e.g., ImagePullResponse.Wait / ImagePushResponse.Wait) correctly surface in-band errors reported via JSON stream messages, and tightens JSON-seq reader behavior to avoid invalid (0, nil) reads.

Changes:

Update Stream.Wait / Stream.JSONMessages to propagate JSON message errors and improve decode/context-cancellation handling (with errdefs-compatible wrapping).
Refine RS filtering reader behavior and adjust NewJSONStreamDecoder to take types.MediaType.
Add targeted unit tests and restore local replace rules + vendor metadata updates.

Reviewed changes

Copilot reviewed 6 out of 11 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
`client/internal/jsonmessages.go`	Returns in-band stream errors from `Wait`, improves decode/cancel error handling, and wraps errors for errdefs classification.
`client/internal/jsonmessages_test.go`	Adds coverage for `Stream.Wait` success/error classification and context-cancel behavior.
`client/internal/json-stream.go`	Improves JSON stream decoder API (MediaType) and fixes RS-filter reader to avoid `(0, nil)` after filtering.
`client/internal/json-stream_test.go`	Expands tests for JSON-seq decoding and RSFilterReader edge-cases.
`go.mod` / `client/go.mod`	Restores `replace` rules to use local `./api`, `./client` (and `../api` within the client submodule).
`go.sum` / `client/go.sum`	Removes sums for locally replaced modules.
`vendor/modules.txt`	Reflects local replaces in vendored module metadata.
`vendor/github.com/moby/moby/client/internal/jsonmessages.go`	Vendored sync of `Stream.Wait` / `JSONMessages` improvements and errdefs wrapping.
`vendor/github.com/moby/moby/client/internal/json-stream.go`	Vendored sync of MediaType decoder signature and RS filter reader fixes.

Comments suppressed due to low confidence (1)

client/internal/json-stream.go:24

NewJSONStreamDecoder now takes types.MediaType, but there are in-repo callers still passing a string (e.g. client/system_events.go uses resp.Header.Get("Content-Type")), which will fail to compile. Update those call-sites (and the vendored equivalents) to pass a types.MediaType value, ideally by parsing the Content-Type header (e.g., strip parameters like ; charset=utf-8) before calling this function so the switch can match the expected media types.

// NewJSONStreamDecoder builds a DecoderFn to read a stream of JSON records
// formatted with the specified content-type.
func NewJSONStreamDecoder(r io.Reader, contentType types.MediaType) DecoderFn {
	switch contentType {
	case types.MediaTypeJSONSequence:
		return json.NewDecoder(NewRSFilterReader(r)).Decode
	case types.MediaTypeJSON, types.MediaTypeNDJSON, types.MediaTypeJSONLines:
		fallthrough
	default:
		return json.NewDecoder(r).Decode

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

The intent of the Wait method is to allow consumers to handle a stream synchronously; block until the push/pull/build completes (or is cancelled) but report errors happening during the stream (e.g. failures to pull the image). Wait previously ignored errors reported via jsonstream.Message.Error, causing push/pull failures to be treated as success. Update it to return these in-band errors while preserving existing handling for decode, transport, and context cancellation errors. Signed-off-by: Sebastiaan van Stijn <[email protected]>

Signed-off-by: Sebastiaan van Stijn <[email protected]>

Just slightly cleaner on scope. Signed-off-by: Sebastiaan van Stijn <[email protected]>

- explicitly return zero-values on error to prevent any potential "garbage" decoded message from being returned. - do not return decoding errors if the context was cancelled; decoding may fail due to the context being cancelled (and reader closed), which could produce unexpected errors. - consolidate all error-handling within a single `err != nil` block. Signed-off-by: Sebastiaan van Stijn <[email protected]>

This buffer was added as part of the initial implementation, but during review it was decided it's not needed; moby#50953 (comment) Remove the buffer as it's not used, and un-export the RSFilterReader implementation. Signed-off-by: Sebastiaan van Stijn <[email protected]>

RSFilterReader could consume input consisting only of RS bytes and return (0, nil), violating io.Reader expectations. This can cause callers to loop indefinitely waiting for progress. For example, a caller like: for { n, err := r.Read(buf) if err != nil { return err } if n == 0 { continue } process(buf[:n]) } would loop forever if Read returns (0, nil) after consuming input. Fix by continuing to read until either filtered data is available or an error (including EOF) is returned. Signed-off-by: Sebastiaan van Stijn <[email protected]>

Also change NewJSONStreamDecoder to use use types.MediaType as argument. The MediaType type-alias that was added in 0e7c817 to help discoverability of accepted media/content-types. Signed-off-by: Sebastiaan van Stijn <[email protected]>

thaJeztah · 2026-04-16T12:01:09Z

cc @vvoland @crazy-max ptal 🤗

Copilot

Pull request overview

Copilot reviewed 4 out of 6 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

thaJeztah added impact/changelog kind/bugfix PR's that fix bugs area/go-sdk impact/go-sdk Noteworthy (compatibility changes) in the Go SDK module/client labels Apr 4, 2026

thaJeztah added this to the 29.4.0 milestone Apr 4, 2026

thaJeztah commented Apr 4, 2026

View reviewed changes

thaJeztah force-pushed the fix_wait branch 2 times, most recently from c8e2e0b to 8aac437 Compare April 5, 2026 12:23

thaJeztah requested review from Copilot and crazy-max April 5, 2026 15:04

Copilot started reviewing on behalf of thaJeztah April 5, 2026 15:05 View session

Copilot AI reviewed Apr 5, 2026

View reviewed changes

vvoland modified the milestones: 29.4.0, 29.4.1 Apr 7, 2026

thaJeztah added 7 commits April 7, 2026 15:44

client/internal: Stream.Wait: return errdefs errors

d5d1801

Signed-off-by: Sebastiaan van Stijn <[email protected]>

client/internal: Stream.JSONMessages: move reader inside the func

5f08008

Just slightly cleaner on scope. Signed-off-by: Sebastiaan van Stijn <[email protected]>

client/internal: touch-up godoc

2601915

Also change NewJSONStreamDecoder to use use types.MediaType as argument. The MediaType type-alias that was added in 0e7c817 to help discoverability of accepted media/content-types. Signed-off-by: Sebastiaan van Stijn <[email protected]>

thaJeztah force-pushed the fix_wait branch from 8aac437 to 2601915 Compare April 7, 2026 13:44

thaJeztah requested a review from vvoland April 8, 2026 10:28

vvoland requested a review from Copilot April 16, 2026 17:19

Copilot started reviewing on behalf of vvoland April 16, 2026 17:20 View session

Copilot AI reviewed Apr 16, 2026

View reviewed changes

vvoland approved these changes Apr 16, 2026

View reviewed changes

thaJeztah merged commit 9291cfe into moby:master Apr 16, 2026
186 checks passed

thaJeztah deleted the fix_wait branch April 16, 2026 19:59

thaJeztah mentioned this pull request Apr 21, 2026

driver/docker-container: remove uses of jsonmessage docker/buildx#3777

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

client/internal: fix Streams.Wait error handling, fix RSFilterReader error handling #52305

client/internal: fix Streams.Wait error handling, fix RSFilterReader error handling #52305
thaJeztah merged 7 commits intomoby:masterfrom
thaJeztah:fix_wait

thaJeztah commented Apr 4, 2026

Uh oh!

thaJeztah Apr 4, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

thaJeztah commented Apr 16, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	func Display(jm jsonstream.Message, out io.Writer, isTerminal bool, width uint16) error {
	if jm.Error != nil {
	return jm.Error
	}

Conversation

thaJeztah commented Apr 4, 2026

go.mod: add back replace rules

client/internal: Stream.Wait: fix missing error-handling

client/internal: Stream.Wait: return errdefs errors

client/internal: Stream.JSONMessages: move reader inside the func

client/internal: Stream.JSONMessages: improve error handling

client/internal: RSFilterReader: un-export, and remove unused buffer

client/internal: improve test-coverage for rsFilterReader

client/internal: fix RSFilterReader returning (0, nil) after filtering

client/internal: touch-up godoc

Uh oh!

thaJeztah Apr 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

thaJeztah commented Apr 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

thaJeztah Apr 4, 2026 •

edited

Loading