From 1d451775871d1a95aa2e52206cf8f7b1a564832d Mon Sep 17 00:00:00 2001 From: Ted Kaplan Date: Thu, 8 Oct 2026 21:43:05 +0000 Subject: [PATCH] Raise errors reported in the stream from ImageCollection.pull The daemon reports a failed pull inside the 200 response's JSON stream, not as an HTTP error. ImageCollection.pull drained that stream without reading it, then looked the image up, so every such failure (a registry refusal, a corrupted layer extraction) surfaced as ImageNotFound for an image that does exist upstream. ContainerCollection.run falls back to pull, so it hid the cause the same way. Decode the stream and raise ImagePullError with the daemon's message, mirroring how ImageCollection.load raises ImageLoadError. Signed-off-by: Ted Kaplan --- docker/errors.py | 4 ++++ docker/models/images.py | 28 ++++++++++++++++++------ tests/unit/models_containers_test.py | 4 +++- tests/unit/models_images_test.py | 32 ++++++++++++++++++++++++---- 4 files changed, 56 insertions(+), 12 deletions(-) diff --git a/docker/errors.py b/docker/errors.py index d7fb78b2bb..b1751cdb20 100644 --- a/docker/errors.py +++ b/docker/errors.py @@ -167,6 +167,10 @@ class ImageLoadError(DockerException): pass +class ImagePullError(DockerException): + pass + + def create_unexpected_kwargs_error(name, kwargs): quoted_kwargs = [f"'{k}'" for k in sorted(kwargs)] text = [f"{name}() "] diff --git a/docker/models/images.py b/docker/models/images.py index 0e8cce3f82..d905d59b1c 100644 --- a/docker/models/images.py +++ b/docker/models/images.py @@ -4,7 +4,12 @@ from ..api import APIClient from ..constants import DEFAULT_DATA_CHUNK_SIZE -from ..errors import BuildError, ImageLoadError, InvalidArgument +from ..errors import ( + BuildError, + ImageLoadError, + ImagePullError, + InvalidArgument, +) from ..utils import parse_repository_tag from ..utils.json_stream import json_stream from .resource import Collection, Model @@ -441,6 +446,9 @@ def pull(self, repository, tag=None, all_tags=False, **kwargs): Raises: :py:class:`docker.errors.APIError` If the server returns an error. + :py:class:`docker.errors.ImagePullError` + If the pull fails after the server accepted the request, + for example because the registry refused it. Example: @@ -462,13 +470,19 @@ def pull(self, repository, tag=None, all_tags=False, **kwargs): del kwargs['stream'] pull_log = self.client.api.pull( - repository, tag=tag, stream=True, all_tags=all_tags, **kwargs + repository, tag=tag, stream=True, decode=True, + all_tags=all_tags, **kwargs ) - for _ in pull_log: - # We don't do anything with the logs, but we need - # to keep the connection alive and wait for the image - # to be pulled. - pass + for chunk in pull_log: + # The daemon answers a failed pull with HTTP 200 and reports the + # failure in the stream, so this is the only place it shows up. + # Without this check the image is simply missing afterwards and + # the caller gets an ImageNotFound that hides the real cause. + if 'errorDetail' in chunk or 'error' in chunk: + detail = chunk.get('errorDetail') or {} + raise ImagePullError( + detail.get('message') or chunk.get('error') + ) if not all_tags: sep = '@' if tag.startswith('sha256:') else ':' return self.get(f'{repository}{sep}{tag}') diff --git a/tests/unit/models_containers_test.py b/tests/unit/models_containers_test.py index 0e2ae341a9..4208c0fcf3 100644 --- a/tests/unit/models_containers_test.py +++ b/tests/unit/models_containers_test.py @@ -257,7 +257,8 @@ def test_run_pull(self): assert container.id == FAKE_CONTAINER_ID client.api.pull.assert_called_with( - 'alpine', platform=None, tag='latest', all_tags=False, stream=True + 'alpine', platform=None, tag='latest', all_tags=False, + stream=True, decode=True, ) def test_run_with_error(self): @@ -354,6 +355,7 @@ def test_run_platform(self): tag='latest', all_tags=False, stream=True, + decode=True, platform='linux/arm64', ) diff --git a/tests/unit/models_images_test.py b/tests/unit/models_images_test.py index 3478c3fedb..19343b0530 100644 --- a/tests/unit/models_images_test.py +++ b/tests/unit/models_images_test.py @@ -1,7 +1,10 @@ import unittest import warnings +import pytest + from docker.constants import DEFAULT_DATA_CHUNK_SIZE +from docker.errors import ImagePullError from docker.models.images import Image from .fake_api import FAKE_IMAGE_ID @@ -46,7 +49,7 @@ def test_pull(self): client = make_fake_client() image = client.images.pull('test_image:test') client.api.pull.assert_called_with( - 'test_image', tag='test', all_tags=False, stream=True + 'test_image', tag='test', all_tags=False, stream=True, decode=True ) client.api.inspect_image.assert_called_with('test_image:test') assert isinstance(image, Image) @@ -56,13 +59,13 @@ def test_pull_tag_precedence(self): client = make_fake_client() image = client.images.pull('test_image:latest', tag='test') client.api.pull.assert_called_with( - 'test_image', tag='test', all_tags=False, stream=True + 'test_image', tag='test', all_tags=False, stream=True, decode=True ) client.api.inspect_image.assert_called_with('test_image:test') image = client.images.pull('test_image') client.api.pull.assert_called_with( - 'test_image', tag='latest', all_tags=False, stream=True + 'test_image', tag='latest', all_tags=False, stream=True, decode=True ) client.api.inspect_image.assert_called_with('test_image:latest') assert isinstance(image, Image) @@ -72,7 +75,7 @@ def test_pull_multiple(self): client = make_fake_client() images = client.images.pull('test_image', all_tags=True) client.api.pull.assert_called_with( - 'test_image', tag='latest', all_tags=True, stream=True + 'test_image', tag='latest', all_tags=True, stream=True, decode=True ) client.api.images.assert_called_with( all=False, name='test_image', filters=None @@ -83,6 +86,27 @@ def test_pull_multiple(self): assert isinstance(image, Image) assert image.id == FAKE_IMAGE_ID + def test_pull_raises_the_error_reported_in_the_stream(self): + client = make_fake_client({ + 'pull.return_value': [ + {'status': 'Pulling from library/test_image'}, + { + 'errorDetail': {'message': 'denied: access forbidden'}, + 'error': 'denied: access forbidden', + }, + ], + }) + with pytest.raises(ImagePullError, match='denied: access forbidden'): + client.images.pull('test_image:test') + client.api.inspect_image.assert_not_called() + + def test_pull_raises_a_bare_error_message(self): + client = make_fake_client({ + 'pull.return_value': [{'error': 'lease does not exist'}], + }) + with pytest.raises(ImagePullError, match='lease does not exist'): + client.images.pull('test_image:test') + def test_pull_with_stream_param(self): client = make_fake_client() with warnings.catch_warnings(record=True) as w: