From 1abaffc41ec97ff7398133890591a17668217de7 Mon Sep 17 00:00:00 2001 From: Haytham Abuelfutuh Date: Tue, 4 Aug 2026 09:30:27 -0700 Subject: [PATCH] fix: surface third-party image builder failures as ImageBuildError (FLYTE-SDK-58) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Builders registered through the `flyte.plugins.image_builders` entry point (or passed directly to `flyte.init(image_builder=...)`) are third-party code. When one raised a bare exception, it travelled all the way out of `ImageBuildEngine.build` unchanged and was recorded as an SDK crash. FLYTE-SDK-58 is exactly that: a plugin builder raised RuntimeError: Failed to get GitHub credentials from git askpass - please reload your terminal or open a new session then try again. which is a message about the user's own machine, not an SDK bug. Wrap exceptions from non-builtin builders as `ImageBuildError` (a `RuntimeUserError`, so `_is_user_error` filters it out of crash reporting) while chaining the original as `__cause__`. Exceptions that already carry a classification — `BaseRuntimeError` and the click control-flow exceptions — are re-raised untouched, and the SDK's own Docker/remote builders are deliberately left unwrapped so genuine SDK failures keep reaching Sentry. Also convert the plugin *load* failure in `_load_custom_image_builders` from a bare `RuntimeError` to `ImageBuildError` for the same reason. fixes FLYTE-SDK-58 Signed-off-by: Haytham Abuelfutuh --- .../_internal/imagebuild/image_builder.py | 38 ++++++- .../imagebuild/test_image_build_engine.py | 102 ++++++++++++++++++ 2 files changed, 138 insertions(+), 2 deletions(-) diff --git a/src/flyte/_internal/imagebuild/image_builder.py b/src/flyte/_internal/imagebuild/image_builder.py index 59728e3dd..90884afc2 100644 --- a/src/flyte/_internal/imagebuild/image_builder.py +++ b/src/flyte/_internal/imagebuild/image_builder.py @@ -214,6 +214,23 @@ class LocalPodmanCommandImageChecker(LocalDockerCommandImageChecker): command_name: ClassVar[str] = "podman" +def _is_builtin_builder(builder: ImageBuilder) -> bool: + """True for the builders shipped with the SDK; anything else is third-party plugin code.""" + from flyte._internal.imagebuild.docker_builder import DockerImageBuilder + from flyte._internal.imagebuild.remote_builder import RemoteImageBuilder + + return isinstance(builder, (DockerImageBuilder, RemoteImageBuilder)) + + +def _is_already_classified(e: Exception) -> bool: + """True when the exception already carries its own user-facing classification.""" + import click + + from flyte.errors import BaseRuntimeError + + return isinstance(e, (BaseRuntimeError, click.Abort, click.exceptions.Exit, click.ClickException)) + + class ImageBuildEngine: """ ImageBuildEngine contains a list of builders that can be used to build an ImageSpec. @@ -333,7 +350,22 @@ async def build( " - flyte.Image(...): pass registry='' (e.g. registry='docker.io/')" ) - result = await img_builder.build_image(image, dry_run=dry_run, wait=wait, force=force) + if _is_builtin_builder(img_builder): + result = await img_builder.build_image(image, dry_run=dry_run, wait=wait, force=force) + else: + # Third-party builders (registered through the `flyte.plugins.image_builders` entry point, or + # passed in directly) are not SDK code. Anything they raise describes the user's build setup, + # not an SDK bug, so surface it as ImageBuildError instead of letting a bare exception escape. + try: + result = await img_builder.build_image(image, dry_run=dry_run, wait=wait, force=force) + except Exception as e: + if _is_already_classified(e): + raise + from flyte.errors import ImageBuildError + + raise ImageBuildError( + f"Image builder `{type(img_builder).__name__}` failed to build {image.uri}: {e}" + ) from e # Persist the freshly built image URI so future runs skip the registry check. # Skip when the build wasn't actually pushed (dry_run) or hasn't finished yet (wait=False). @@ -373,7 +405,9 @@ def _load_custom_image_builders(cls, name: str) -> ImageBuilder: return builder() return builder except Exception as e: - raise RuntimeError(f"Failed to load image builder {ep.name} with error: {e}") + from flyte.errors import ImageBuildError + + raise ImageBuildError(f"Failed to load image builder {ep.name} with error: {e}") from e raise ValueError( f"Unknown image builder type: {name}. Available builders:" f" {[ep.name for ep in plugins] + ['local', 'remote']}" diff --git a/tests/flyte/imagebuild/test_image_build_engine.py b/tests/flyte/imagebuild/test_image_build_engine.py index 44747f2ca..afe36399e 100644 --- a/tests/flyte/imagebuild/test_image_build_engine.py +++ b/tests/flyte/imagebuild/test_image_build_engine.py @@ -396,3 +396,105 @@ async def test_local_docker_checker_unexpected_error_raises(mock_exec): mock_exec.return_value = _make_mock_process(1, b"", b"connection refused") with pytest.raises(RuntimeError, match="Failed to run docker buildx imagetools inspect"): await LocalDockerCommandImageChecker.image_exists("registry/img", "tag1") + + +def _plugin_builder(exc): + """A third-party builder (not one of the SDK's own) whose build_image raises ``exc``.""" + + class _PluginBuilder: + async def build_image(self, image, dry_run, wait=True, force=False): + raise exc + + def get_checkers(self): + return None + + return _PluginBuilder() + + +@mock.patch("flyte._internal.imagebuild.image_builder.ImageBuildEngine._get_builder") +@mock.patch("flyte._internal.imagebuild.image_builder.ImageBuildEngine.image_exists", new_callable=mock.AsyncMock) +@pytest.mark.asyncio +async def test_plugin_builder_failure_becomes_image_build_error(mock_image_exists, mock_get_builder): + """A bare exception from a third-party builder is surfaced as ImageBuildError. + + Regression for FLYTE-SDK-58: a plugin builder registered through the + `flyte.plugins.image_builders` entry point raised + ``RuntimeError("Failed to get GitHub credentials from git askpass ...")`` — a message about the + user's own environment — which escaped the engine untouched and was reported as an SDK crash. + """ + from flyte._sentry import _is_user_error + from flyte.errors import ImageBuildError + + ImageBuildEngine.build.cache_clear() + mock_image_exists.return_value = None + original = RuntimeError("Failed to get GitHub credentials from git askpass - please reload your terminal") + mock_get_builder.return_value = _plugin_builder(original) + + img = Image.from_base("ghcr.io/example/base:latest").clone(name="my-app", extendable=True) + with pytest.raises(ImageBuildError, match="failed to build") as exc_info: + await ImageBuildEngine.build(image=img, builder="remote") + + # The original message survives, and the cause chain is kept for debugging. + assert "git askpass" in str(exc_info.value) + assert exc_info.value.__cause__ is original + # The point of the exercise: it no longer looks like an SDK crash to Sentry. + assert _is_user_error(exc_info.value) + + +@mock.patch("flyte._internal.imagebuild.image_builder.ImageBuildEngine._get_builder") +@mock.patch("flyte._internal.imagebuild.image_builder.ImageBuildEngine.image_exists", new_callable=mock.AsyncMock) +@pytest.mark.asyncio +async def test_plugin_builder_classified_errors_pass_through(mock_image_exists, mock_get_builder): + """Exceptions that already carry a classification are re-raised as-is, not double-wrapped.""" + import click + + from flyte.errors import ImageBuildError + + for original in (ImageBuildError("already classified"), click.Abort(), click.ClickException("bad flag")): + ImageBuildEngine.build.cache_clear() + mock_image_exists.return_value = None + mock_get_builder.return_value = _plugin_builder(original) + + img = Image.from_base("ghcr.io/example/base:latest").clone(name="my-app", extendable=True) + with pytest.raises(type(original)) as exc_info: + await ImageBuildEngine.build(image=img, builder="remote") + assert exc_info.value is original + + +@mock.patch("flyte._internal.imagebuild.image_builder.ImageBuildEngine.image_exists", new_callable=mock.AsyncMock) +@pytest.mark.asyncio +async def test_builtin_builder_failure_is_not_wrapped(mock_image_exists): + """Failures in the SDK's own builders are real bugs — they must keep reaching crash reporting.""" + from flyte._internal.imagebuild.remote_builder import RemoteImageBuilder + + ImageBuildEngine.build.cache_clear() + mock_image_exists.return_value = None + original = RuntimeError("boom inside the SDK builder") + + img = Image.from_base("ghcr.io/example/base:latest").clone(name="my-app", extendable=True) + with mock.patch.object(RemoteImageBuilder, "build_image", new_callable=mock.AsyncMock) as mock_build: + mock_build.side_effect = original + with pytest.raises(RuntimeError) as exc_info: + await ImageBuildEngine.build(image=img, builder="remote") + + assert exc_info.value is original + + +def test_custom_builder_load_failure_raises_image_build_error(): + """A plugin that fails to import is a user-environment problem, not an SDK crash.""" + from flyte._sentry import _is_user_error + from flyte.errors import ImageBuildError + + class _FailingEntryPoint: + name = "my-builder" + + @staticmethod + def load(): + raise ImportError("No module named 'my_builder_deps'") + + with mock.patch("flyte._internal.imagebuild.image_builder.entry_points", return_value=[_FailingEntryPoint()]): + with pytest.raises(ImageBuildError, match="Failed to load image builder my-builder") as exc_info: + ImageBuildEngine._get_builder("my-builder") + + assert isinstance(exc_info.value.__cause__, ImportError) + assert _is_user_error(exc_info.value)