From 54aed55e919a013036ffcd2416b711ba683bdd07 Mon Sep 17 00:00:00 2001 From: Madhav Donthula Date: Fri, 17 Jul 2026 15:51:38 -0700 Subject: [PATCH 01/22] feat: add --output json flag to sam build for structured machine-readable output Add a --output option to `sam build` that supports "text" (default, unchanged behavior) and "json" (structured output for programmatic consumers). When --output json is specified: - Success output is a JSON object with status, build_dir, template_file, and a resources array listing each built function's logical_id, runtime, and architecture. - Error output is a JSON object with status, error type, message, and the failing resource name when available. - Build progress logs are suppressed from stdout (sent to stderr only) so that stdout contains only valid JSON. This enables CI/CD pipelines, IDE extensions, and AI-assisted developer tools to consume build results programmatically without fragile text parsing. --- samcli/commands/build/build_context.py | 67 ++++++++++++++++++++++---- samcli/commands/build/command.py | 11 +++++ samcli/commands/build/core/options.py | 2 +- samcli/lib/build/app_builder.py | 30 +++++++----- samcli/lib/build/exceptions.py | 5 +- 5 files changed, 90 insertions(+), 25 deletions(-) diff --git a/samcli/commands/build/build_context.py b/samcli/commands/build/build_context.py index fd8285accb2..8851670bf06 100644 --- a/samcli/commands/build/build_context.py +++ b/samcli/commands/build/build_context.py @@ -4,10 +4,12 @@ import copy import itertools +import json import logging import os import pathlib import shutil +import sys from collections import Counter from typing import Any, Dict, List, Optional, Tuple @@ -104,6 +106,7 @@ def __init__( mount_symlinks: Optional[bool] = False, use_buildkit: Optional[bool] = False, language_extensions: Optional[bool] = None, + output: str = "text", ) -> None: """ Initialize the class @@ -213,6 +216,7 @@ def __init__( self._mount_symlinks = mount_symlinks self._use_buildkit = use_buildkit self._language_extensions_enabled = resolve_language_extensions_enabled(language_extensions) + self._output = output def __enter__(self) -> "BuildContext": self.set_up() @@ -269,6 +273,9 @@ def get_resources_to_build(self): def run(self) -> None: """Runs the building process by creating an ApplicationBuilder.""" + if self._output == "json": + logging.getLogger("samcli").setLevel(logging.WARNING) + if self._is_sam_template(): SamApiProvider.check_implicit_api_resource_ids(self.stacks) @@ -321,8 +328,6 @@ def run(self) -> None: self._handle_build_post_processing(builder, self._build_result) - click.secho("\nBuild Succeeded", fg="green") - # try to use relpath so the command is easier to understand, however, # under Windows, when SAM and (build_dir or output_template_path) are # on different drive, relpath() fails. @@ -336,17 +341,43 @@ def run(self) -> None: build_dir_in_success_message = self.build_dir output_template_path_in_success_message = out_template_path - if self._print_success_message: - msg = self._gen_success_msg( - build_dir_in_success_message, - output_template_path_in_success_message, - os.path.abspath(self.build_dir) == os.path.abspath(DEFAULT_BUILD_DIR), - ) - - click.secho(msg, fg="yellow") + if self._output == "json": + result = { + "status": "success", + "build_dir": build_dir_in_success_message, + "template_file": output_template_path_in_success_message, + "resources": [ + { + "logical_id": f.full_path, + "runtime": f.runtime, + "architecture": f.architectures[0] if f.architectures else None, + } + for f in self.get_resources_to_build().functions + ], + } + click.echo(json.dumps(result, indent=2)) + else: + click.secho("\nBuild Succeeded", fg="green") + if self._print_success_message: + msg = self._gen_success_msg( + build_dir_in_success_message, + output_template_path_in_success_message, + os.path.abspath(self.build_dir) == os.path.abspath(DEFAULT_BUILD_DIR), + ) + click.secho(msg, fg="yellow") except FunctionNotFound as function_not_found_ex: caught_exception = function_not_found_ex + if self._output == "json": + error_result = { + "status": "failure", + "error": { + "type": "FunctionNotFound", + "message": str(function_not_found_ex), + }, + } + click.echo(json.dumps(error_result, indent=2)) + sys.exit(1) raise UserException( str(function_not_found_ex), wrapped_from=function_not_found_ex.__class__.__name__ ) from function_not_found_ex @@ -360,6 +391,22 @@ def run(self) -> None: ) as ex: caught_exception = ex + if self._output == "json": + deep_wrap = getattr(ex, "wrapped_from", None) + error_type = deep_wrap if deep_wrap else ex.__class__.__name__ + error_result = { + "status": "failure", + "error": { + "type": error_type, + "message": str(ex), + }, + } + resource_name = getattr(ex, "resource_name", None) + if resource_name: + error_result["error"]["resource"] = resource_name + click.echo(json.dumps(error_result, indent=2)) + sys.exit(1) + click.secho("\nBuild Failed", fg="red") # Some Exceptions have a deeper wrapped exception that needs to be surfaced diff --git a/samcli/commands/build/command.py b/samcli/commands/build/command.py index 8ec13607e70..cea6c08e1a5 100644 --- a/samcli/commands/build/command.py +++ b/samcli/commands/build/command.py @@ -129,6 +129,13 @@ @template_option_without_build @parameter_override_option @docker_common_options +@click.option( + "--output", + default="text", + help="Output the results from the command in a given output format. " + "Supported formats: text (default), json.", + type=click.Choice(["text", "json"], case_sensitive=False), +) @cli_framework_options @aws_creds_options @click.argument("resource_logical_id", required=False) @@ -167,6 +174,7 @@ def cli( mount_symlinks: Optional[bool], use_buildkit: Optional[bool], language_extensions: Optional[bool], + output: str, ) -> None: """ `sam build` command entry point @@ -201,6 +209,7 @@ def cli( mount_symlinks, use_buildkit, language_extensions, + output, ) # pragma: no cover @@ -230,6 +239,7 @@ def do_cli( # pylint: disable=too-many-locals, too-many-statements mount_symlinks: Optional[bool], use_buildkit: Optional[bool], language_extensions: Optional[bool], + output: str = "text", ) -> None: """ Implementation of the ``cli`` method @@ -272,6 +282,7 @@ def do_cli( # pylint: disable=too-many-locals, too-many-statements mount_symlinks=mount_symlinks, use_buildkit=use_buildkit, language_extensions=language_extensions, + output=output, ) as ctx: ctx.run() diff --git a/samcli/commands/build/core/options.py b/samcli/commands/build/core/options.py index 4b86570817f..01d6af38d09 100644 --- a/samcli/commands/build/core/options.py +++ b/samcli/commands/build/core/options.py @@ -30,7 +30,7 @@ EXTENSION_OPTIONS: List[str] = ["hook_name", "skip_prepare_infra"] -BUILD_STRATEGY_OPTIONS: List[str] = ["parallel", "exclude", "manifest", "cached", "build_in_source"] +BUILD_STRATEGY_OPTIONS: List[str] = ["parallel", "exclude", "manifest", "cached", "build_in_source", "output"] ARTIFACT_LOCATION_OPTIONS: List[str] = [ "build_dir", diff --git a/samcli/lib/build/app_builder.py b/samcli/lib/build/app_builder.py index c30c948c007..aae3dcd0243 100644 --- a/samcli/lib/build/app_builder.py +++ b/samcli/lib/build/app_builder.py @@ -807,19 +807,23 @@ def _build_function( # pylint: disable=R1710 specified_workflow=specified_workflow if supported_specified_workflow else None, ) - return self._build_function_in_process( - config, - code_dir, - artifact_dir, - scratch_dir, - manifest_path, - runtime, - architecture, - options, - dependencies_dir, - download_dependencies, - self._combine_dependencies, - ) + try: + return self._build_function_in_process( + config, + code_dir, + artifact_dir, + scratch_dir, + manifest_path, + runtime, + architecture, + options, + dependencies_dir, + download_dependencies, + self._combine_dependencies, + ) + except BuildError as ex: + ex.resource_name = function_name + raise # pylint: disable=fixme # FIXME: we need to throw an exception here, packagetype could be something else diff --git a/samcli/lib/build/exceptions.py b/samcli/lib/build/exceptions.py index e1f4ff50fcc..1526501f7c1 100644 --- a/samcli/lib/build/exceptions.py +++ b/samcli/lib/build/exceptions.py @@ -2,6 +2,8 @@ Build Related Exceptions. """ +from typing import Optional + from samcli.commands.exceptions import UserException @@ -15,8 +17,9 @@ def __init__(self, container_name: str, error_msg: str) -> None: class BuildError(Exception): - def __init__(self, wrapped_from: str, msg: str) -> None: + def __init__(self, wrapped_from: str, msg: str, resource_name: Optional[str] = None) -> None: self.wrapped_from = wrapped_from + self.resource_name = resource_name Exception.__init__(self, msg) From 2fd6125a594d4a4d019664d1fbc1175b55e4faa8 Mon Sep 17 00:00:00 2001 From: Madhav Donthula Date: Wed, 22 Jul 2026 11:49:51 -0700 Subject: [PATCH 02/22] fix: address code review feedback on --output json - Replace sys.exit(1) with raise UserException in error handlers to preserve telemetry tracking via @track_command decorator - Wrap both container and in-process build paths in try/except so resource_name is tagged on BuildError regardless of build mode - Include layers in JSON success output alongside functions - Rename field from logical_id to resource_id (accurate for nested stacks) - Add type field ("function" or "layer") to each resource entry --- samcli/commands/build/build_context.py | 32 +++++++++++++--------- samcli/lib/build/app_builder.py | 38 +++++++++++++------------- 2 files changed, 38 insertions(+), 32 deletions(-) diff --git a/samcli/commands/build/build_context.py b/samcli/commands/build/build_context.py index 8851670bf06..8e9f61666ec 100644 --- a/samcli/commands/build/build_context.py +++ b/samcli/commands/build/build_context.py @@ -9,7 +9,6 @@ import os import pathlib import shutil -import sys from collections import Counter from typing import Any, Dict, List, Optional, Tuple @@ -342,18 +341,27 @@ def run(self) -> None: output_template_path_in_success_message = out_template_path if self._output == "json": + resources = [ + { + "resource_id": f.full_path, + "type": "function", + "runtime": f.runtime, + "architecture": f.architectures[0] if f.architectures else None, + } + for f in self.get_resources_to_build().functions + ] + [ + { + "resource_id": layer.full_path, + "type": "layer", + "compatible_runtimes": layer.compatible_runtimes, + } + for layer in self.get_resources_to_build().layers + ] result = { "status": "success", "build_dir": build_dir_in_success_message, "template_file": output_template_path_in_success_message, - "resources": [ - { - "logical_id": f.full_path, - "runtime": f.runtime, - "architecture": f.architectures[0] if f.architectures else None, - } - for f in self.get_resources_to_build().functions - ], + "resources": resources, } click.echo(json.dumps(result, indent=2)) else: @@ -377,7 +385,6 @@ def run(self) -> None: }, } click.echo(json.dumps(error_result, indent=2)) - sys.exit(1) raise UserException( str(function_not_found_ex), wrapped_from=function_not_found_ex.__class__.__name__ ) from function_not_found_ex @@ -405,9 +412,8 @@ def run(self) -> None: if resource_name: error_result["error"]["resource"] = resource_name click.echo(json.dumps(error_result, indent=2)) - sys.exit(1) - - click.secho("\nBuild Failed", fg="red") + else: + click.secho("\nBuild Failed", fg="red") # Some Exceptions have a deeper wrapped exception that needs to be surfaced # from deeper than just one level down. diff --git a/samcli/lib/build/app_builder.py b/samcli/lib/build/app_builder.py index aae3dcd0243..ae3bb7f07f6 100644 --- a/samcli/lib/build/app_builder.py +++ b/samcli/lib/build/app_builder.py @@ -788,26 +788,26 @@ def _build_function( # pylint: disable=R1710 scratch_dir=scratch_dir_path, ) # By default prefer to build in-process for speed - if self._container_manager: - # None represents the global build image for all functions/layers - global_image = self._build_images.get(None) - image = self._build_images.get(function_name, global_image) - # pass to container only when specified workflow is supported to overwrite runtime to get image - supported_specified_workflow = supports_specified_workflow(specified_workflow) - return self._build_function_on_container( - config, - code_dir, - artifact_dir, - manifest_path, - runtime, - architecture, - options, - container_env_vars, - image, - specified_workflow=specified_workflow if supported_specified_workflow else None, - ) - try: + if self._container_manager: + # None represents the global build image for all functions/layers + global_image = self._build_images.get(None) + image = self._build_images.get(function_name, global_image) + # pass to container only when specified workflow is supported to overwrite runtime to get image + supported_specified_workflow = supports_specified_workflow(specified_workflow) + return self._build_function_on_container( + config, + code_dir, + artifact_dir, + manifest_path, + runtime, + architecture, + options, + container_env_vars, + image, + specified_workflow=specified_workflow if supported_specified_workflow else None, + ) + return self._build_function_in_process( config, code_dir, From b104652e5e3f7e21587aefc0b27865e7b5d82470 Mon Sep 17 00:00:00 2001 From: Madhav Donthula Date: Wed, 22 Jul 2026 16:22:38 -0700 Subject: [PATCH 03/22] fix: move resource_name tagging to strategy layer for universal coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the try/except BuildError from app_builder._build_function (which only covered the ZIP in-process path) to DefaultBuildStrategy's build_single_function_definition and build_single_layer_definition. This ensures resource_name is tagged for ALL build paths: - ZIP functions (in-process and container) - IMAGE functions (Docker builds) - Layer builds Known limitation: BuildInsideContainerError does not inherit from BuildError (it inherits from Exception directly), so it won't get resource_name tagged by this mechanism. Fixing this would require changing the exception hierarchy which is out of scope for this PR. The error type and message are still present in JSON output for this case — only the resource field is missing. Also fixes unit test assertion to expect the new output="text" kwarg. --- samcli/lib/build/app_builder.py | 52 +++++++++----------- samcli/lib/build/build_strategy.py | 24 ++++++++- tests/unit/commands/buildcmd/test_command.py | 1 + 3 files changed, 48 insertions(+), 29 deletions(-) diff --git a/samcli/lib/build/app_builder.py b/samcli/lib/build/app_builder.py index ae3bb7f07f6..c30c948c007 100644 --- a/samcli/lib/build/app_builder.py +++ b/samcli/lib/build/app_builder.py @@ -788,42 +788,38 @@ def _build_function( # pylint: disable=R1710 scratch_dir=scratch_dir_path, ) # By default prefer to build in-process for speed - try: - if self._container_manager: - # None represents the global build image for all functions/layers - global_image = self._build_images.get(None) - image = self._build_images.get(function_name, global_image) - # pass to container only when specified workflow is supported to overwrite runtime to get image - supported_specified_workflow = supports_specified_workflow(specified_workflow) - return self._build_function_on_container( - config, - code_dir, - artifact_dir, - manifest_path, - runtime, - architecture, - options, - container_env_vars, - image, - specified_workflow=specified_workflow if supported_specified_workflow else None, - ) - - return self._build_function_in_process( + if self._container_manager: + # None represents the global build image for all functions/layers + global_image = self._build_images.get(None) + image = self._build_images.get(function_name, global_image) + # pass to container only when specified workflow is supported to overwrite runtime to get image + supported_specified_workflow = supports_specified_workflow(specified_workflow) + return self._build_function_on_container( config, code_dir, artifact_dir, - scratch_dir, manifest_path, runtime, architecture, options, - dependencies_dir, - download_dependencies, - self._combine_dependencies, + container_env_vars, + image, + specified_workflow=specified_workflow if supported_specified_workflow else None, ) - except BuildError as ex: - ex.resource_name = function_name - raise + + return self._build_function_in_process( + config, + code_dir, + artifact_dir, + scratch_dir, + manifest_path, + runtime, + architecture, + options, + dependencies_dir, + download_dependencies, + self._combine_dependencies, + ) # pylint: disable=fixme # FIXME: we need to throw an exception here, packagetype could be something else diff --git a/samcli/lib/build/build_strategy.py b/samcli/lib/build/build_strategy.py index 87b412b41c5..45c78908bb9 100644 --- a/samcli/lib/build/build_strategy.py +++ b/samcli/lib/build/build_strategy.py @@ -20,7 +20,7 @@ LayerBuildDefinition, ) from samcli.lib.build.dependency_hash_generator import DependencyHashGenerator -from samcli.lib.build.exceptions import MissingBuildMethodException +from samcli.lib.build.exceptions import BuildError, MissingBuildMethodException from samcli.lib.build.utils import warn_on_invalid_architecture from samcli.lib.utils import osutils from samcli.lib.utils.architecture import X86_64 @@ -144,6 +144,17 @@ def build_single_function_definition(self, build_definition: FunctionBuildDefini """ Build the unique definition and then copy the artifact to the corresponding function folder """ + try: + return self._do_build_single_function_definition(build_definition) + except BuildError as ex: + if ex.resource_name is None: + ex.resource_name = build_definition.get_full_path() + raise + + def _do_build_single_function_definition(self, build_definition: FunctionBuildDefinition) -> Dict[str, str]: + """ + Internal implementation of build_single_function_definition + """ function_build_results = {} LOG.info( "Building codeuri: %s runtime: %s architecture: %s functions: %s", @@ -210,6 +221,17 @@ def build_single_layer_definition(self, layer_definition: LayerBuildDefinition) """ Build the unique definition and then copy the artifact to the corresponding layer folder """ + try: + return self._do_build_single_layer_definition(layer_definition) + except BuildError as ex: + if ex.resource_name is None: + ex.resource_name = layer_definition.full_path + raise + + def _do_build_single_layer_definition(self, layer_definition: LayerBuildDefinition) -> Dict[str, str]: + """ + Internal implementation of build_single_layer_definition + """ layer = layer_definition.layer LOG.info("Building layer '%s'", layer.full_path) if layer.build_method is None: diff --git a/tests/unit/commands/buildcmd/test_command.py b/tests/unit/commands/buildcmd/test_command.py index d32b27bf6f5..babc5a58bc6 100644 --- a/tests/unit/commands/buildcmd/test_command.py +++ b/tests/unit/commands/buildcmd/test_command.py @@ -70,6 +70,7 @@ def test_must_succeed_build(self, os_mock, BuildContextMock, mock_build_click): mount_symlinks=True, use_buildkit=False, language_extensions=None, + output="text", ) ctx_mock.run.assert_called_with() self.assertEqual(ctx_mock.run.call_count, 1) From c5a0769c4072e2fe623b1b9f8398319a6c4f0dae Mon Sep 17 00:00:00 2001 From: Madhav Donthula Date: Wed, 22 Jul 2026 16:33:18 -0700 Subject: [PATCH 04/22] fix: broaden exception catch to tag resource_name for all build error types Expand the except clause in DefaultBuildStrategy to also catch UnsupportedRuntimeException, BuildInsideContainerError, and UnsupportedBuilderLibraryVersionError. These exceptions don't inherit from BuildError but can originate from a specific function build (deprecated runtime, Docker pull failure, missing builder in container). Uses dynamic attribute assignment (ex.resource_name = ...) since these classes don't define resource_name in __init__. The consumer in build_context.py already uses getattr(ex, "resource_name", None) which handles both cases safely. --- samcli/lib/build/build_strategy.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/samcli/lib/build/build_strategy.py b/samcli/lib/build/build_strategy.py index 45c78908bb9..6a049dc3ac0 100644 --- a/samcli/lib/build/build_strategy.py +++ b/samcli/lib/build/build_strategy.py @@ -20,7 +20,8 @@ LayerBuildDefinition, ) from samcli.lib.build.dependency_hash_generator import DependencyHashGenerator -from samcli.lib.build.exceptions import BuildError, MissingBuildMethodException +from samcli.lib.build.exceptions import BuildError, BuildInsideContainerError, MissingBuildMethodException, UnsupportedBuilderLibraryVersionError +from samcli.lib.build.workflow_config import UnsupportedRuntimeException from samcli.lib.build.utils import warn_on_invalid_architecture from samcli.lib.utils import osutils from samcli.lib.utils.architecture import X86_64 @@ -146,8 +147,8 @@ def build_single_function_definition(self, build_definition: FunctionBuildDefini """ try: return self._do_build_single_function_definition(build_definition) - except BuildError as ex: - if ex.resource_name is None: + except (BuildError, UnsupportedRuntimeException, BuildInsideContainerError, UnsupportedBuilderLibraryVersionError) as ex: + if getattr(ex, "resource_name", None) is None: ex.resource_name = build_definition.get_full_path() raise @@ -223,8 +224,8 @@ def build_single_layer_definition(self, layer_definition: LayerBuildDefinition) """ try: return self._do_build_single_layer_definition(layer_definition) - except BuildError as ex: - if ex.resource_name is None: + except (BuildError, UnsupportedRuntimeException, BuildInsideContainerError, UnsupportedBuilderLibraryVersionError) as ex: + if getattr(ex, "resource_name", None) is None: ex.resource_name = layer_definition.full_path raise From 4e8752fab8888f7a2a674f06987b223fc81047b6 Mon Sep 17 00:00:00 2001 From: Madhav Donthula Date: Thu, 23 Jul 2026 11:59:11 -0700 Subject: [PATCH 05/22] fix: formatting, lint, and schema for CI checks - Apply black formatting to command.py and build_strategy.py - Fix ruff import ordering in build_strategy.py - Add type: ignore comments for mypy union-attr and index errors (dynamic resource_name assignment on non-BuildError exceptions) - Regenerate schema/samcli.json to include the new --output parameter --- samcli/commands/build/build_context.py | 2 +- samcli/commands/build/command.py | 3 +-- samcli/lib/build/build_strategy.py | 27 ++++++++++++++++++++------ schema/samcli.json | 12 +++++++++++- 4 files changed, 34 insertions(+), 10 deletions(-) diff --git a/samcli/commands/build/build_context.py b/samcli/commands/build/build_context.py index 8e9f61666ec..d6f88cfb649 100644 --- a/samcli/commands/build/build_context.py +++ b/samcli/commands/build/build_context.py @@ -410,7 +410,7 @@ def run(self) -> None: } resource_name = getattr(ex, "resource_name", None) if resource_name: - error_result["error"]["resource"] = resource_name + error_result["error"]["resource"] = resource_name # type: ignore[index] click.echo(json.dumps(error_result, indent=2)) else: click.secho("\nBuild Failed", fg="red") diff --git a/samcli/commands/build/command.py b/samcli/commands/build/command.py index cea6c08e1a5..c5825a00344 100644 --- a/samcli/commands/build/command.py +++ b/samcli/commands/build/command.py @@ -132,8 +132,7 @@ @click.option( "--output", default="text", - help="Output the results from the command in a given output format. " - "Supported formats: text (default), json.", + help="Output the results from the command in a given output format. " "Supported formats: text (default), json.", type=click.Choice(["text", "json"], case_sensitive=False), ) @cli_framework_options diff --git a/samcli/lib/build/build_strategy.py b/samcli/lib/build/build_strategy.py index 6a049dc3ac0..ebe36d7acc8 100644 --- a/samcli/lib/build/build_strategy.py +++ b/samcli/lib/build/build_strategy.py @@ -20,9 +20,14 @@ LayerBuildDefinition, ) from samcli.lib.build.dependency_hash_generator import DependencyHashGenerator -from samcli.lib.build.exceptions import BuildError, BuildInsideContainerError, MissingBuildMethodException, UnsupportedBuilderLibraryVersionError -from samcli.lib.build.workflow_config import UnsupportedRuntimeException +from samcli.lib.build.exceptions import ( + BuildError, + BuildInsideContainerError, + MissingBuildMethodException, + UnsupportedBuilderLibraryVersionError, +) from samcli.lib.build.utils import warn_on_invalid_architecture +from samcli.lib.build.workflow_config import UnsupportedRuntimeException from samcli.lib.utils import osutils from samcli.lib.utils.architecture import X86_64 from samcli.lib.utils.async_utils import AsyncContext @@ -147,9 +152,14 @@ def build_single_function_definition(self, build_definition: FunctionBuildDefini """ try: return self._do_build_single_function_definition(build_definition) - except (BuildError, UnsupportedRuntimeException, BuildInsideContainerError, UnsupportedBuilderLibraryVersionError) as ex: + except ( + BuildError, + UnsupportedRuntimeException, + BuildInsideContainerError, + UnsupportedBuilderLibraryVersionError, + ) as ex: if getattr(ex, "resource_name", None) is None: - ex.resource_name = build_definition.get_full_path() + ex.resource_name = build_definition.get_full_path() # type: ignore[union-attr] raise def _do_build_single_function_definition(self, build_definition: FunctionBuildDefinition) -> Dict[str, str]: @@ -224,9 +234,14 @@ def build_single_layer_definition(self, layer_definition: LayerBuildDefinition) """ try: return self._do_build_single_layer_definition(layer_definition) - except (BuildError, UnsupportedRuntimeException, BuildInsideContainerError, UnsupportedBuilderLibraryVersionError) as ex: + except ( + BuildError, + UnsupportedRuntimeException, + BuildInsideContainerError, + UnsupportedBuilderLibraryVersionError, + ) as ex: if getattr(ex, "resource_name", None) is None: - ex.resource_name = layer_definition.full_path + ex.resource_name = layer_definition.full_path # type: ignore[union-attr] raise def _do_build_single_layer_definition(self, layer_definition: LayerBuildDefinition) -> Dict[str, str]: diff --git a/schema/samcli.json b/schema/samcli.json index 40498eb619e..a4d17a0117c 100644 --- a/schema/samcli.json +++ b/schema/samcli.json @@ -300,7 +300,7 @@ "properties": { "parameters": { "title": "Parameters for the build command", - "description": "Available parameters for the build command:\n* terraform_project_root_path:\nUsed for passing the Terraform project root directory path. Current directory will be used as a default value, if this parameter is not provided.\n* hook_name:\nHook package id to extend AWS SAM CLI commands functionality. \n\nExample: `terraform` to extend AWS SAM CLI commands functionality to support terraform applications. \n\nAvailable Hook Names: ['terraform']\n* skip_prepare_infra:\nSkip preparation stage when there are no infrastructure changes. Only used in conjunction with --hook-name.\n* use_container:\nBuild functions within an AWS Lambda-like container.\n* use_buildkit:\nEnable buildkit for container image builds. Requires Docker with buildx plugin or Finch CLI.\n* build_in_source:\nOpts in to build project in the source folder. The following workflows support building in source: ['nodejs16.x', 'nodejs18.x', 'nodejs20.x', 'nodejs22.x', 'Makefile', 'esbuild']\n* language_extensions:\nExpand AWS::LanguageExtensions transforms (Fn::ForEach, Fn::Length, Fn::ToJsonString, Fn::FindInMap with DefaultValue) locally before running SAM transforms. Off by default. Equivalent env var: SAM_CLI_ENABLE_LANGUAGE_EXTENSIONS=1.\n* container_env_var:\nEnvironment variables to be passed into build containers\nResource format (FuncName.VarName=Value) or Global format (VarName=Value).\n\n Example: --container-env-var Func1.VAR1=value1 --container-env-var VAR2=value2\n* container_env_var_file:\nEnvironment variables json file (e.g., env_vars.json) to be passed to containers.\n* build_image:\nContainer image URIs for building functions/layers. You can specify for all functions/layers with just the image URI (--build-image public.ecr.aws/sam/build-nodejs18.x:latest). You can specify for each individual function with (--build-image FunctionLogicalID=public.ecr.aws/sam/build-nodejs18.x:latest). A combination of the two can be used. If a function does not have build image specified or an image URI for all functions, the default SAM CLI build images will be used.\n* exclude:\nName of the resource(s) to exclude from AWS SAM CLI build.\n* parallel:\nEnable parallel builds for AWS SAM template's functions and layers.\n* mount_with:\nSpecify mount mode for building functions/layers inside container. If it is mounted with write permissions, some files in source code directory may be changed/added by the build process. By default the source code directory is read only.\n* mount_symlinks:\nSpecify if symlinks at the top level of the code should be mounted inside the container. Activating this flag could allow access to locations outside of your workspace by using a symbolic link. By default symlinks are not mounted.\n* build_dir:\nDirectory to store build artifacts.Note: This directory will be first removed before starting a build.\n* cache_dir:\nDirectory to store cached artifacts. The default cache directory is .aws-sam/cache\n* base_dir:\nResolve relative paths to function's source code with respect to this directory. Use this if SAM template and source code are not in same enclosing folder. By default, relative paths are resolved with respect to the SAM template's location.\n* manifest:\nPath to a custom dependency manifest. Example: custom-package.json\n* cached:\nEnable cached builds.Reuse build artifacts that have not changed from previous builds. \n\nAWS SAM CLI evaluates if files in your project directory have changed. \n\nNote: AWS SAM CLI does not evaluate changes made to third party modules that the project depends on.Example: Python function includes a requirements.txt file with the following entry requests=1.x and the latest request module version changes from 1.1 to 1.2, AWS SAM CLI will not pull the latest version until a non-cached build is run.\n* template_file:\nAWS SAM template file.\n* parameter_overrides:\nString that contains AWS CloudFormation parameter overrides encoded as key=value pairs.\n* skip_pull_image:\nSkip pulling down the latest Docker image for Lambda runtime.\n* docker_network:\nName or ID of an existing docker network for AWS Lambda docker containers to connect to, along with the default bridge network. If not specified, the Lambda containers will only connect to the default bridge docker network.\n* beta_features:\nEnable/Disable beta features.\n* debug:\nTurn on debug logging to print debug message generated by AWS SAM CLI and display timestamps.\n* profile:\nSelect a specific profile from your credential file to get AWS credentials.\n* region:\nSet the AWS Region of the service. (e.g. us-east-1)\n* save_params:\nSave the parameters provided via the command line to the configuration file.", + "description": "Available parameters for the build command:\n* terraform_project_root_path:\nUsed for passing the Terraform project root directory path. Current directory will be used as a default value, if this parameter is not provided.\n* hook_name:\nHook package id to extend AWS SAM CLI commands functionality. \n\nExample: `terraform` to extend AWS SAM CLI commands functionality to support terraform applications. \n\nAvailable Hook Names: ['terraform']\n* skip_prepare_infra:\nSkip preparation stage when there are no infrastructure changes. Only used in conjunction with --hook-name.\n* use_container:\nBuild functions within an AWS Lambda-like container.\n* use_buildkit:\nEnable buildkit for container image builds. Requires Docker with buildx plugin or Finch CLI.\n* build_in_source:\nOpts in to build project in the source folder. The following workflows support building in source: ['nodejs16.x', 'nodejs18.x', 'nodejs20.x', 'nodejs22.x', 'Makefile', 'esbuild']\n* language_extensions:\nExpand AWS::LanguageExtensions transforms (Fn::ForEach, Fn::Length, Fn::ToJsonString, Fn::FindInMap with DefaultValue) locally before running SAM transforms. Off by default. Equivalent env var: SAM_CLI_ENABLE_LANGUAGE_EXTENSIONS=1.\n* container_env_var:\nEnvironment variables to be passed into build containers\nResource format (FuncName.VarName=Value) or Global format (VarName=Value).\n\n Example: --container-env-var Func1.VAR1=value1 --container-env-var VAR2=value2\n* container_env_var_file:\nEnvironment variables json file (e.g., env_vars.json) to be passed to containers.\n* build_image:\nContainer image URIs for building functions/layers. You can specify for all functions/layers with just the image URI (--build-image public.ecr.aws/sam/build-nodejs18.x:latest). You can specify for each individual function with (--build-image FunctionLogicalID=public.ecr.aws/sam/build-nodejs18.x:latest). A combination of the two can be used. If a function does not have build image specified or an image URI for all functions, the default SAM CLI build images will be used.\n* exclude:\nName of the resource(s) to exclude from AWS SAM CLI build.\n* parallel:\nEnable parallel builds for AWS SAM template's functions and layers.\n* mount_with:\nSpecify mount mode for building functions/layers inside container. If it is mounted with write permissions, some files in source code directory may be changed/added by the build process. By default the source code directory is read only.\n* mount_symlinks:\nSpecify if symlinks at the top level of the code should be mounted inside the container. Activating this flag could allow access to locations outside of your workspace by using a symbolic link. By default symlinks are not mounted.\n* build_dir:\nDirectory to store build artifacts.Note: This directory will be first removed before starting a build.\n* cache_dir:\nDirectory to store cached artifacts. The default cache directory is .aws-sam/cache\n* base_dir:\nResolve relative paths to function's source code with respect to this directory. Use this if SAM template and source code are not in same enclosing folder. By default, relative paths are resolved with respect to the SAM template's location.\n* manifest:\nPath to a custom dependency manifest. Example: custom-package.json\n* cached:\nEnable cached builds.Reuse build artifacts that have not changed from previous builds. \n\nAWS SAM CLI evaluates if files in your project directory have changed. \n\nNote: AWS SAM CLI does not evaluate changes made to third party modules that the project depends on.Example: Python function includes a requirements.txt file with the following entry requests=1.x and the latest request module version changes from 1.1 to 1.2, AWS SAM CLI will not pull the latest version until a non-cached build is run.\n* template_file:\nAWS SAM template file.\n* parameter_overrides:\nString that contains AWS CloudFormation parameter overrides encoded as key=value pairs.\n* skip_pull_image:\nSkip pulling down the latest Docker image for Lambda runtime.\n* docker_network:\nName or ID of an existing docker network for AWS Lambda docker containers to connect to, along with the default bridge network. If not specified, the Lambda containers will only connect to the default bridge docker network.\n* output:\nOutput the results from the command in a given output format. Supported formats: text (default), json.\n* beta_features:\nEnable/Disable beta features.\n* debug:\nTurn on debug logging to print debug message generated by AWS SAM CLI and display timestamps.\n* profile:\nSelect a specific profile from your credential file to get AWS credentials.\n* region:\nSet the AWS Region of the service. (e.g. us-east-1)\n* save_params:\nSave the parameters provided via the command line to the configuration file.", "type": "object", "properties": { "terraform_project_root_path": { @@ -433,6 +433,16 @@ "type": "string", "description": "Name or ID of an existing docker network for AWS Lambda docker containers to connect to, along with the default bridge network. If not specified, the Lambda containers will only connect to the default bridge docker network." }, + "output": { + "title": "output", + "type": "string", + "description": "Output the results from the command in a given output format. Supported formats: text (default), json.", + "default": "text", + "enum": [ + "json", + "text" + ] + }, "beta_features": { "title": "beta_features", "type": "boolean", From 127c008a7f75ffa8fd8528833fb20b6c7e1e1876 Mon Sep 17 00:00:00 2001 From: Madhav Donthula Date: Thu, 23 Jul 2026 14:17:20 -0700 Subject: [PATCH 06/22] fix: add output param to samconfig test assertions --- tests/unit/commands/samconfig/test_samconfig.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/unit/commands/samconfig/test_samconfig.py b/tests/unit/commands/samconfig/test_samconfig.py index 86f189a3f10..fb859a2a193 100644 --- a/tests/unit/commands/samconfig/test_samconfig.py +++ b/tests/unit/commands/samconfig/test_samconfig.py @@ -166,6 +166,7 @@ def test_build(self, do_cli_mock): True, False, None, + "text", ) @patch("samcli.commands.build.command.do_cli") @@ -228,6 +229,7 @@ def test_build_with_no_use_container(self, do_cli_mock): False, False, None, + "text", ) @patch("samcli.commands.build.command.do_cli") @@ -289,6 +291,7 @@ def test_build_with_no_use_container_option(self, do_cli_mock): False, False, None, + "text", ) @patch("samcli.commands.build.command.do_cli") @@ -351,6 +354,7 @@ def test_build_with_no_use_container_override(self, do_cli_mock): False, False, None, + "text", ) @patch("samcli.commands.build.command.do_cli") @@ -414,6 +418,7 @@ def test_build_with_no_cached_override(self, do_cli_mock): False, False, None, + "text", ) @patch("samcli.commands.build.command.do_cli") @@ -474,6 +479,7 @@ def test_build_with_container_env_vars(self, do_cli_mock): False, False, None, + "text", ) @patch("samcli.commands.build.command.do_cli") @@ -533,6 +539,7 @@ def test_build_with_build_images(self, do_cli_mock): False, False, None, + "text", ) @patch("samcli.commands.local.invoke.cli.do_cli") From de503bf28e156fea5885d7bf4e3e5e6626654bff Mon Sep 17 00:00:00 2001 From: Madhav Donthula Date: Thu, 23 Jul 2026 14:33:44 -0700 Subject: [PATCH 07/22] fix: remove unnecessary log suppression (logs go to stderr, not stdout) --- samcli/commands/build/build_context.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/samcli/commands/build/build_context.py b/samcli/commands/build/build_context.py index d6f88cfb649..9e3441056b3 100644 --- a/samcli/commands/build/build_context.py +++ b/samcli/commands/build/build_context.py @@ -272,9 +272,6 @@ def get_resources_to_build(self): def run(self) -> None: """Runs the building process by creating an ApplicationBuilder.""" - if self._output == "json": - logging.getLogger("samcli").setLevel(logging.WARNING) - if self._is_sam_template(): SamApiProvider.check_implicit_api_resource_ids(self.stacks) From 514edc27e265ce52a53a98560b99458902485135 Mon Sep 17 00:00:00 2001 From: Madhav Donthula Date: Thu, 30 Jul 2026 15:19:27 -0700 Subject: [PATCH 08/22] refactor(build): address review feedback on --output json - Move --output into a shared @output_option decorator in _utils/options.py so deploy/init can reuse it instead of redeclaring the option - Give --output its own "Output Options" help group rather than filing it under Build Strategy, which is for options that change how the build runs - Extract the JSON/text reporting out of BuildContext.run() into _print_build_success and _print_build_failure - Collect get_resources_to_build() once in run() and reuse it - Always emit the "resource" key in error output (null when the failure is not attributable to a single resource) so the schema is predictable - Hoist the wrapped_from lookup to the top of the except block instead of computing it in both branches - Drop the unused resource_name param from BuildError; build_strategy sets the attribute at runtime for all handled exception types --- samcli/commands/_utils/options.py | 14 +++ samcli/commands/build/build_context.py | 159 +++++++++++++++---------- samcli/commands/build/command.py | 8 +- samcli/commands/build/core/options.py | 6 +- samcli/lib/build/exceptions.py | 5 +- 5 files changed, 119 insertions(+), 73 deletions(-) diff --git a/samcli/commands/_utils/options.py b/samcli/commands/_utils/options.py index 4801f5bab37..4edc300bcf4 100644 --- a/samcli/commands/_utils/options.py +++ b/samcli/commands/_utils/options.py @@ -447,6 +447,20 @@ def common_observability_options(f): return f +def output_click_option(): + return click.option( + "--output", + default="text", + help="Output the results from the command in a given output format. " + "Supported formats: text (default), json.", + type=click.Choice(["text", "json"], case_sensitive=False), + ) + + +def output_option(f): + return output_click_option()(f) + + def metadata_click_option(): return click.option( "--metadata", diff --git a/samcli/commands/build/build_context.py b/samcli/commands/build/build_context.py index 9e3441056b3..1412e601e14 100644 --- a/samcli/commands/build/build_context.py +++ b/samcli/commands/build/build_context.py @@ -280,6 +280,8 @@ def run(self) -> None: caught_exception: Optional[Exception] = None try: + resources_to_build = self.get_resources_to_build() + # boolean value indicates if mount with write or not, defaults to READ ONLY mount_with_write = False if self._use_container: @@ -289,12 +291,12 @@ def run(self) -> None: # if self._mount_with is NOT WRITE # check the need of mounting with write permissions and prompt user to enable it if needed mount_with_write = prompt_user_to_enable_mount_with_write_if_needed( - self.get_resources_to_build(), + resources_to_build, self.base_dir, ) builder = ApplicationBuilder( - self.get_resources_to_build(), + resources_to_build, self.build_dir, self.base_dir, self.cache_dir, @@ -317,7 +319,7 @@ def run(self) -> None: self._check_exclude_warning() self._check_build_method_experimental_flag() - for f in self.get_resources_to_build().functions: + for f in resources_to_build.functions: EventTracker.track_event(EventName.BUILD_FUNCTION_RUNTIME.value, f.runtime) self._build_result = builder.build() @@ -337,51 +339,15 @@ def run(self) -> None: build_dir_in_success_message = self.build_dir output_template_path_in_success_message = out_template_path - if self._output == "json": - resources = [ - { - "resource_id": f.full_path, - "type": "function", - "runtime": f.runtime, - "architecture": f.architectures[0] if f.architectures else None, - } - for f in self.get_resources_to_build().functions - ] + [ - { - "resource_id": layer.full_path, - "type": "layer", - "compatible_runtimes": layer.compatible_runtimes, - } - for layer in self.get_resources_to_build().layers - ] - result = { - "status": "success", - "build_dir": build_dir_in_success_message, - "template_file": output_template_path_in_success_message, - "resources": resources, - } - click.echo(json.dumps(result, indent=2)) - else: - click.secho("\nBuild Succeeded", fg="green") - if self._print_success_message: - msg = self._gen_success_msg( - build_dir_in_success_message, - output_template_path_in_success_message, - os.path.abspath(self.build_dir) == os.path.abspath(DEFAULT_BUILD_DIR), - ) - click.secho(msg, fg="yellow") + self._print_build_success( + build_dir_in_success_message, + output_template_path_in_success_message, + resources_to_build, + ) except FunctionNotFound as function_not_found_ex: caught_exception = function_not_found_ex - if self._output == "json": - error_result = { - "status": "failure", - "error": { - "type": "FunctionNotFound", - "message": str(function_not_found_ex), - }, - } - click.echo(json.dumps(error_result, indent=2)) + self._print_build_failure(function_not_found_ex, "FunctionNotFound", print_text_banner=False) raise UserException( str(function_not_found_ex), wrapped_from=function_not_found_ex.__class__.__name__ ) from function_not_found_ex @@ -395,27 +361,13 @@ def run(self) -> None: ) as ex: caught_exception = ex - if self._output == "json": - deep_wrap = getattr(ex, "wrapped_from", None) - error_type = deep_wrap if deep_wrap else ex.__class__.__name__ - error_result = { - "status": "failure", - "error": { - "type": error_type, - "message": str(ex), - }, - } - resource_name = getattr(ex, "resource_name", None) - if resource_name: - error_result["error"]["resource"] = resource_name # type: ignore[index] - click.echo(json.dumps(error_result, indent=2)) - else: - click.secho("\nBuild Failed", fg="red") - # Some Exceptions have a deeper wrapped exception that needs to be surfaced # from deeper than just one level down. deep_wrap = getattr(ex, "wrapped_from", None) wrapped_from = deep_wrap if deep_wrap else ex.__class__.__name__ + + self._print_build_failure(ex, wrapped_from) + raise UserException(str(ex), wrapped_from=wrapped_from) from ex finally: if self.build_in_source: @@ -1126,6 +1078,89 @@ def _copy_artifact_paths(self, original_resource: Dict, modified_resource: Dict) if value is not None: _set_prop_value(original_props, prop_name, value) + def _print_build_success( + self, artifacts_dir: str, output_template_path: str, resources_to_build: ResourcesToBuildCollector + ) -> None: + """ + Reports a successful build, either as structured JSON or as a human readable message. + + Parameters + ---------- + artifacts_dir: str + A string path representing the folder of built artifacts + output_template_path: str + A string path representing the final template file + resources_to_build: ResourcesToBuildCollector + The functions and layers that were built + """ + if self._output == "json": + resources: List[Dict[str, Any]] = [ + { + "resource_id": function.full_path, + "type": "function", + "runtime": function.runtime, + "architecture": function.architectures[0] if function.architectures else None, + } + for function in resources_to_build.functions + ] + [ + { + "resource_id": layer.full_path, + "type": "layer", + "compatible_runtimes": layer.compatible_runtimes, + } + for layer in resources_to_build.layers + ] + click.echo( + json.dumps( + { + "status": "success", + "build_dir": artifacts_dir, + "template_file": output_template_path, + "resources": resources, + }, + indent=2, + ) + ) + return + + click.secho("\nBuild Succeeded", fg="green") + if self._print_success_message: + msg = self._gen_success_msg( + artifacts_dir, + output_template_path, + os.path.abspath(self.build_dir) == os.path.abspath(DEFAULT_BUILD_DIR), + ) + click.secho(msg, fg="yellow") + + def _print_build_failure(self, ex: Exception, error_type: str, print_text_banner: bool = True) -> None: + """ + Reports a failed build, either as structured JSON or as a human readable message. + + Parameters + ---------- + ex: Exception + The exception that caused the build to fail + error_type: str + The error type to report + print_text_banner: bool + Whether to print the "Build Failed" banner in text mode + """ + if self._output != "json": + if print_text_banner: + click.secho("\nBuild Failed", fg="red") + return + + # The resource key is always present so consumers can rely on the schema. It is + # populated from the exception when the failure is attributable to a specific + # resource, otherwise from the resource the customer asked to build (which is the + # relevant name even when that resource does not exist in the template). It is + # None when the failure is not tied to any single resource. + resource_name = getattr(ex, "resource_name", None) or self._resource_identifier + + error: Dict[str, Any] = {"type": error_type, "message": str(ex), "resource": resource_name} + + click.echo(json.dumps({"status": "failure", "error": error}, indent=2)) + def _gen_success_msg(self, artifacts_dir: str, output_template_path: str, is_default_build_dir: bool) -> str: """ Generates a success message containing some suggested commands to run diff --git a/samcli/commands/build/command.py b/samcli/commands/build/command.py index c5825a00344..78e0fb9acad 100644 --- a/samcli/commands/build/command.py +++ b/samcli/commands/build/command.py @@ -26,6 +26,7 @@ language_extensions_option, manifest_option, mount_symlinks_option, + output_option, parameter_override_option, skip_prepare_infra_option, template_option_without_build, @@ -129,12 +130,7 @@ @template_option_without_build @parameter_override_option @docker_common_options -@click.option( - "--output", - default="text", - help="Output the results from the command in a given output format. " "Supported formats: text (default), json.", - type=click.Choice(["text", "json"], case_sensitive=False), -) +@output_option @cli_framework_options @aws_creds_options @click.argument("resource_logical_id", required=False) diff --git a/samcli/commands/build/core/options.py b/samcli/commands/build/core/options.py index 01d6af38d09..4c4da7c134f 100644 --- a/samcli/commands/build/core/options.py +++ b/samcli/commands/build/core/options.py @@ -30,7 +30,7 @@ EXTENSION_OPTIONS: List[str] = ["hook_name", "skip_prepare_infra"] -BUILD_STRATEGY_OPTIONS: List[str] = ["parallel", "exclude", "manifest", "cached", "build_in_source", "output"] +BUILD_STRATEGY_OPTIONS: List[str] = ["parallel", "exclude", "manifest", "cached", "build_in_source"] ARTIFACT_LOCATION_OPTIONS: List[str] = [ "build_dir", @@ -38,6 +38,8 @@ "base_dir", ] +OUTPUT_OPTIONS: List[str] = ["output"] + TEMPLATE_OPTIONS: List[str] = ["parameter_overrides", "language_extensions"] TERRAFORM_HOOK_OPTIONS: List[str] = ["terraform_project_root_path"] @@ -49,6 +51,7 @@ + BUILD_STRATEGY_OPTIONS + CONTAINER_OPTION_NAMES + ARTIFACT_LOCATION_OPTIONS + + OUTPUT_OPTIONS + EXTENSION_OPTIONS + CONFIGURATION_OPTION_NAMES + ALL_COMMON_OPTIONS @@ -66,6 +69,7 @@ "Artifact Location Options": { "option_names": {opt: {"rank": idx} for idx, opt in enumerate(ARTIFACT_LOCATION_OPTIONS)} }, + "Output Options": {"option_names": {opt: {"rank": idx} for idx, opt in enumerate(OUTPUT_OPTIONS)}}, "Extension Options": {"option_names": {opt: {"rank": idx} for idx, opt in enumerate(EXTENSION_OPTIONS)}}, "Configuration Options": { "option_names": {opt: {"rank": idx} for idx, opt in enumerate(CONFIGURATION_OPTION_NAMES)}, diff --git a/samcli/lib/build/exceptions.py b/samcli/lib/build/exceptions.py index 1526501f7c1..e1f4ff50fcc 100644 --- a/samcli/lib/build/exceptions.py +++ b/samcli/lib/build/exceptions.py @@ -2,8 +2,6 @@ Build Related Exceptions. """ -from typing import Optional - from samcli.commands.exceptions import UserException @@ -17,9 +15,8 @@ def __init__(self, container_name: str, error_msg: str) -> None: class BuildError(Exception): - def __init__(self, wrapped_from: str, msg: str, resource_name: Optional[str] = None) -> None: + def __init__(self, wrapped_from: str, msg: str) -> None: self.wrapped_from = wrapped_from - self.resource_name = resource_name Exception.__init__(self, msg) From 0bd5c26adfa79e3dd2e9c0ebbd1825cfb0da0fee Mon Sep 17 00:00:00 2001 From: Madhav Donthula Date: Fri, 31 Jul 2026 13:16:22 -0700 Subject: [PATCH 09/22] test(build): add unit tests for --output json reporting methods Cover _print_build_success and _print_build_failure across JSON and text modes: resource list shapes for functions/layers, architecture edge cases, the always-present "resource" error key, error-type passthrough, and the text-banner suppression path. 23 tests following the existing TestBuildContext_ convention. --- .../commands/buildcmd/test_build_context.py | 261 +++++++++++++++++- 1 file changed, 260 insertions(+), 1 deletion(-) diff --git a/tests/unit/commands/buildcmd/test_build_context.py b/tests/unit/commands/buildcmd/test_build_context.py index eab988f231e..125e4f7b249 100644 --- a/tests/unit/commands/buildcmd/test_build_context.py +++ b/tests/unit/commands/buildcmd/test_build_context.py @@ -1,3 +1,4 @@ +import json import os from unittest import TestCase from unittest.mock import ANY, MagicMock, Mock, call, patch @@ -17,7 +18,7 @@ from samcli.lib.build.build_graph import DEFAULT_DEPENDENCIES_DIR from samcli.lib.build.bundler import EsbuildBundlerManager from samcli.lib.build.workflow_config import UnsupportedRuntimeException -from samcli.lib.providers.provider import Function, get_function_build_info +from samcli.lib.providers.provider import Function, ResourcesToBuildCollector, get_function_build_info from samcli.lib.telemetry.event import EventName, UsedFeature from samcli.lib.utils.osutils import BUILD_DIR_PERMISSIONS from samcli.lib.utils.packagetype import IMAGE, ZIP @@ -1411,6 +1412,264 @@ def test_gen_message_with_non_default_build_with_hook_package(self): self.assertEqual(msg, expected_msg) +class TestBuildContext_print_build_success(TestCase): + def setUp(self): + self.build_dir = ".aws-sam/build" + self.template_file = "template_file" + + self.build_context = BuildContext( + resource_identifier="function_identifier", + template_file=self.template_file, + base_dir="base_dir", + build_dir=self.build_dir, + cache_dir="cache_dir", + parallel=False, + mode="mode", + cached=False, + output="json", + ) + self.build_context._hook_name = False + + @staticmethod + def _collector(functions=None, layers=None): + collector = ResourcesToBuildCollector() + collector.add_functions(functions or []) + collector.add_layers(layers or []) + return collector + + @patch("samcli.commands.build.build_context.click.secho") + @patch("samcli.commands.build.build_context.click.echo") + def test_json_with_functions_and_layers(self, echo_mock, secho_mock): + collector = self._collector( + functions=[get_function("Fn", runtime="python3.12")], + layers=[DummyLayer("Lyr", "python3.12")], + ) + collector.layers[0].full_path = "Lyr" + collector.layers[0].compatible_runtimes = ["python3.12", "python3.11"] + + self.build_context._print_build_success("artifacts", "out_template", collector) + + # JSON goes to click.echo; text-mode secho must not be used + self.assertEqual(echo_mock.call_count, 1) + secho_mock.assert_not_called() + + result = json.loads(echo_mock.call_args[0][0]) + self.assertEqual(result["status"], "success") + self.assertEqual(result["build_dir"], "artifacts") + self.assertEqual(result["template_file"], "out_template") + # Order is functions first, then layers + self.assertEqual(result["resources"][0]["type"], "function") + self.assertEqual(result["resources"][0]["resource_id"], "Fn") + self.assertEqual(result["resources"][1]["type"], "layer") + self.assertEqual(result["resources"][1]["resource_id"], "Lyr") + self.assertEqual(result["resources"][1]["compatible_runtimes"], ["python3.12", "python3.11"]) + + @patch("samcli.commands.build.build_context.click.echo") + def test_json_functions_only(self, echo_mock): + collector = self._collector(functions=[get_function("Fn", runtime="python3.12")]) + + self.build_context._print_build_success("artifacts", "out_template", collector) + + result = json.loads(echo_mock.call_args[0][0]) + self.assertEqual(len(result["resources"]), 1) + self.assertEqual(result["resources"][0]["type"], "function") + + @patch("samcli.commands.build.build_context.click.echo") + def test_json_layers_only(self, echo_mock): + layer = DummyLayer("Lyr", "python3.12") + layer.full_path = "Lyr" + layer.compatible_runtimes = ["python3.12"] + collector = self._collector(layers=[layer]) + + self.build_context._print_build_success("artifacts", "out_template", collector) + + result = json.loads(echo_mock.call_args[0][0]) + self.assertEqual(len(result["resources"]), 1) + self.assertEqual(result["resources"][0]["type"], "layer") + + @patch("samcli.commands.build.build_context.click.echo") + def test_json_empty_collector(self, echo_mock): + self.build_context._print_build_success("artifacts", "out_template", self._collector()) + + result = json.loads(echo_mock.call_args[0][0]) + self.assertEqual(result["resources"], []) + + @patch("samcli.commands.build.build_context.click.echo") + def test_json_architecture_defaults_to_none_when_absent(self, echo_mock): + # get_function() builds a Function with architectures=None + collector = self._collector(functions=[get_function("Fn", runtime="python3.12")]) + + self.build_context._print_build_success("artifacts", "out_template", collector) + + result = json.loads(echo_mock.call_args[0][0]) + self.assertIsNone(result["resources"][0]["architecture"]) + + @patch("samcli.commands.build.build_context.click.echo") + def test_json_multi_architecture_reports_first(self, echo_mock): + # Function is an immutable namedtuple, so build a variant with _replace + function = get_function("Fn", runtime="python3.12")._replace(architectures=["arm64", "x86_64"]) + collector = self._collector(functions=[function]) + + self.build_context._print_build_success("artifacts", "out_template", collector) + + result = json.loads(echo_mock.call_args[0][0]) + self.assertEqual(result["resources"][0]["architecture"], "arm64") + + @patch("samcli.commands.build.build_context.click.echo") + def test_json_emitted_even_when_success_message_disabled(self, echo_mock): + self.build_context._print_success_message = False + collector = self._collector(functions=[get_function("Fn", runtime="python3.12")]) + + self.build_context._print_build_success("artifacts", "out_template", collector) + + result = json.loads(echo_mock.call_args[0][0]) + self.assertEqual(result["status"], "success") + + @patch("samcli.commands.build.build_context.click.echo") + def test_json_uses_indentation(self, echo_mock): + self.build_context._print_build_success("artifacts", "out_template", self._collector()) + + self.assertIn('\n "status"', echo_mock.call_args[0][0]) + + @patch("samcli.commands.build.build_context.click.secho") + @patch("samcli.commands.build.build_context.click.echo") + def test_text_mode_prints_banner_and_message(self, echo_mock, secho_mock): + self.build_context._output = "text" + collector = self._collector(functions=[get_function("Fn", runtime="python3.12")]) + + self.build_context._print_build_success("artifacts", "out_template", collector) + + # Text mode uses secho, never echo + echo_mock.assert_not_called() + secho_mock.assert_any_call("\nBuild Succeeded", fg="green") + self.assertEqual(secho_mock.call_count, 2) + + @patch("samcli.commands.build.build_context.click.secho") + @patch("samcli.commands.build.build_context.click.echo") + def test_text_mode_banner_only_when_success_message_disabled(self, echo_mock, secho_mock): + self.build_context._output = "text" + self.build_context._print_success_message = False + + self.build_context._print_build_success("artifacts", "out_template", self._collector()) + + echo_mock.assert_not_called() + secho_mock.assert_called_once_with("\nBuild Succeeded", fg="green") + + +class TestBuildContext_print_build_failure(TestCase): + def setUp(self): + self.build_context = BuildContext( + resource_identifier="function_identifier", + template_file="template_file", + base_dir="base_dir", + build_dir="build_dir", + cache_dir="cache_dir", + parallel=False, + mode="mode", + cached=False, + output="json", + ) + + @patch("samcli.commands.build.build_context.click.echo") + def test_json_resource_from_exception(self, echo_mock): + ex = BuildError(wrapped_from="WorkflowFailedError", msg="dependency failure") + ex.resource_name = "HelloWorldFunction" + + self.build_context._print_build_failure(ex, "WorkflowFailedError") + + error = json.loads(echo_mock.call_args[0][0])["error"] + self.assertEqual(error["resource"], "HelloWorldFunction") + + @patch("samcli.commands.build.build_context.click.echo") + def test_json_resource_falls_back_to_resource_identifier(self, echo_mock): + # ResourceNotFound carries no resource_name, so the resource the user asked + # to build is used - even though it does not exist in the template. + ex = ResourceNotFound("Unable to find a function or layer with name 'function_identifier'") + + self.build_context._print_build_failure(ex, "ResourceNotFound") + + error = json.loads(echo_mock.call_args[0][0])["error"] + self.assertEqual(error["resource"], "function_identifier") + + @patch("samcli.commands.build.build_context.click.echo") + def test_json_resource_is_null_when_unattributable(self, echo_mock): + self.build_context._resource_identifier = None + ex = ResourceNotFound("no resource") + + self.build_context._print_build_failure(ex, "ResourceNotFound") + + error = json.loads(echo_mock.call_args[0][0])["error"] + # The key is always present so consumers can rely on the schema + self.assertIn("resource", error) + self.assertIsNone(error["resource"]) + + @patch("samcli.commands.build.build_context.click.echo") + def test_json_exception_resource_name_wins_over_identifier(self, echo_mock): + ex = BuildError(wrapped_from="BuildError", msg="msg") + ex.resource_name = "FromException" + + self.build_context._print_build_failure(ex, "BuildError") + + error = json.loads(echo_mock.call_args[0][0])["error"] + self.assertEqual(error["resource"], "FromException") + + @patch("samcli.commands.build.build_context.click.echo") + def test_json_error_shape(self, echo_mock): + ex = BuildError(wrapped_from="BuildError", msg="something broke") + + self.build_context._print_build_failure(ex, "BuildError") + + result = json.loads(echo_mock.call_args[0][0]) + self.assertEqual(result["status"], "failure") + self.assertEqual(set(result["error"].keys()), {"type", "message", "resource"}) + self.assertEqual(result["error"]["type"], "BuildError") + self.assertEqual(result["error"]["message"], "something broke") + + @parameterized.expand( + [ + ("FunctionNotFound",), + ("UnsupportedRuntimeException",), + ("BuildInsideContainerError",), + ("InvalidBuildGraphException",), + ("ResourceNotFound",), + ] + ) + @patch("samcli.commands.build.build_context.click.echo") + def test_json_error_type_passed_through(self, error_type, echo_mock): + self.build_context._print_build_failure(BuildError("wrap", "msg"), error_type) + + result = json.loads(echo_mock.call_args[0][0]) + self.assertEqual(result["error"]["type"], error_type) + + @patch("samcli.commands.build.build_context.click.secho") + @patch("samcli.commands.build.build_context.click.echo") + def test_json_emitted_even_when_text_banner_suppressed(self, echo_mock, secho_mock): + self.build_context._print_build_failure(FunctionNotFound("nope"), "FunctionNotFound", print_text_banner=False) + + self.assertEqual(echo_mock.call_count, 1) + secho_mock.assert_not_called() + + @patch("samcli.commands.build.build_context.click.secho") + @patch("samcli.commands.build.build_context.click.echo") + def test_text_mode_prints_banner(self, echo_mock, secho_mock): + self.build_context._output = "text" + + self.build_context._print_build_failure(BuildError("wrap", "msg"), "BuildError") + + echo_mock.assert_not_called() + secho_mock.assert_called_once_with("\nBuild Failed", fg="red") + + @patch("samcli.commands.build.build_context.click.secho") + @patch("samcli.commands.build.build_context.click.echo") + def test_text_mode_prints_nothing_when_banner_suppressed(self, echo_mock, secho_mock): + self.build_context._output = "text" + + self.build_context._print_build_failure(FunctionNotFound("nope"), "FunctionNotFound", print_text_banner=False) + + echo_mock.assert_not_called() + secho_mock.assert_not_called() + + class TestBuildContext_check_build_method_experimental_flag(TestCase): def setUp(self): self.build_context = BuildContext( From 5d85acdd73fb208c240570e4ddf4b18b61141edf Mon Sep 17 00:00:00 2001 From: Madhav Donthula Date: Wed, 5 Aug 2026 12:44:33 -0700 Subject: [PATCH 10/22] fix: address code review feedback on --output json - Rename output helpers to structured_output_option (avoid shadowing list command) - Use OutputOption enum instead of string literals - Emit JSON error for InvalidBuildDirException; soften "always present" comment - Single-line JSON (remove indent=2) - Add resource_name class attribute to all 4 build exception types, drop type: ignore - Add end-to-end run() JSON test; remove 6 redundant helper tests - Use function.architecture (resolves x86_64 default) instead of architectures[0] - Route interactive prompts to stderr (err=True) so stdout stays pure JSON --- samcli/commands/_utils/experimental.py | 2 +- samcli/commands/_utils/options.py | 11 +- samcli/commands/build/build_context.py | 27 ++-- samcli/commands/build/command.py | 81 +++++++----- samcli/commands/build/utils.py | 3 +- samcli/lib/build/build_strategy.py | 4 +- samcli/lib/build/exceptions.py | 8 +- samcli/lib/build/workflow_config.py | 2 +- .../unit/commands/_utils/test_experimental.py | 2 +- .../commands/buildcmd/test_build_context.py | 116 ++++++++---------- tests/unit/commands/buildcmd/test_command.py | 2 + 11 files changed, 142 insertions(+), 116 deletions(-) diff --git a/samcli/commands/_utils/experimental.py b/samcli/commands/_utils/experimental.py index 871fdb4db15..34b1f7e4d34 100644 --- a/samcli/commands/_utils/experimental.py +++ b/samcli/commands/_utils/experimental.py @@ -272,7 +272,7 @@ def prompt_experimental( if is_experimental_enabled(config_entry): update_experimental_context() return True - confirmed = click.confirm(Colored().yellow(prompt), default=False) + confirmed = click.confirm(Colored().yellow(prompt), default=False, err=True) if confirmed: set_experimental(config_entry=config_entry, enabled=True) update_experimental_context() diff --git a/samcli/commands/_utils/options.py b/samcli/commands/_utils/options.py index 4edc300bcf4..dca8fe26a56 100644 --- a/samcli/commands/_utils/options.py +++ b/samcli/commands/_utils/options.py @@ -447,7 +447,12 @@ def common_observability_options(f): return f -def output_click_option(): +def structured_output_click_option(): + """Shared --output option for commands that support structured (JSON) output. + + Uses text|json choices matching the OutputOption enum in samcli.lib.observability.util. + Intended as the single shared contract for all commands adopting --output json. + """ return click.option( "--output", default="text", @@ -457,8 +462,8 @@ def output_click_option(): ) -def output_option(f): - return output_click_option()(f) +def structured_output_option(f): + return structured_output_click_option()(f) def metadata_click_option(): diff --git a/samcli/commands/build/build_context.py b/samcli/commands/build/build_context.py index 1412e601e14..0d273f58d09 100644 --- a/samcli/commands/build/build_context.py +++ b/samcli/commands/build/build_context.py @@ -48,6 +48,7 @@ ) from samcli.lib.cfn_language_extensions.utils import is_foreach_key from samcli.lib.intrinsic_resolver.intrinsics_symbol_table import IntrinsicsSymbolTable +from samcli.lib.observability.util import OutputOption from samcli.lib.package.language_extensions_packaging import ( _get_prop_value, _leaf_prop_name, @@ -215,7 +216,7 @@ def __init__( self._mount_symlinks = mount_symlinks self._use_buildkit = use_buildkit self._language_extensions_enabled = resolve_language_extensions_enabled(language_extensions) - self._output = output + self._output = OutputOption(output) def __enter__(self) -> "BuildContext": self.set_up() @@ -1093,13 +1094,13 @@ def _print_build_success( resources_to_build: ResourcesToBuildCollector The functions and layers that were built """ - if self._output == "json": + if self._output is OutputOption.json: resources: List[Dict[str, Any]] = [ { "resource_id": function.full_path, "type": "function", "runtime": function.runtime, - "architecture": function.architectures[0] if function.architectures else None, + "architecture": function.architecture, } for function in resources_to_build.functions ] + [ @@ -1117,8 +1118,7 @@ def _print_build_success( "build_dir": artifacts_dir, "template_file": output_template_path, "resources": resources, - }, - indent=2, + } ) ) return @@ -1145,21 +1145,21 @@ def _print_build_failure(self, ex: Exception, error_type: str, print_text_banner print_text_banner: bool Whether to print the "Build Failed" banner in text mode """ - if self._output != "json": + if self._output is not OutputOption.json: if print_text_banner: click.secho("\nBuild Failed", fg="red") return - # The resource key is always present so consumers can rely on the schema. It is + # The resource key is included in the error JSON when available. It is # populated from the exception when the failure is attributable to a specific - # resource, otherwise from the resource the customer asked to build (which is the - # relevant name even when that resource does not exist in the template). It is - # None when the failure is not tied to any single resource. + # resource, or from the resource the customer asked to build. It may be + # None when the failure is not tied to any single resource (e.g. invalid + # build directory, template parse errors). resource_name = getattr(ex, "resource_name", None) or self._resource_identifier error: Dict[str, Any] = {"type": error_type, "message": str(ex), "resource": resource_name} - click.echo(json.dumps({"status": "failure", "error": error}, indent=2)) + click.echo(json.dumps({"status": "failure", "error": error})) def _gen_success_msg(self, artifacts_dir: str, output_template_path: str, is_default_build_dir: bool) -> str: """ @@ -1230,7 +1230,10 @@ def _setup_build_dir(build_dir: str, clean: bool) -> str: # build folder contains something inside. Clear everything. shutil.rmtree(build_dir) - build_path.mkdir(mode=BUILD_DIR_PERMISSIONS, parents=True, exist_ok=True) + try: + build_path.mkdir(mode=BUILD_DIR_PERMISSIONS, parents=True, exist_ok=True) + except OSError as ex: + raise InvalidBuildDirException(f"Unable to use build dir {build_dir}. Reason: {str(ex)}") from ex # ensure path resolving is done after creation: https://bugs.python.org/issue32434 return str(build_path.resolve()) diff --git a/samcli/commands/build/command.py b/samcli/commands/build/command.py index 78e0fb9acad..47dfea3d11c 100644 --- a/samcli/commands/build/command.py +++ b/samcli/commands/build/command.py @@ -26,9 +26,9 @@ language_extensions_option, manifest_option, mount_symlinks_option, - output_option, parameter_override_option, skip_prepare_infra_option, + structured_output_option, template_option_without_build, terraform_project_root_path_option, use_buildkit_option, @@ -130,7 +130,7 @@ @template_option_without_build @parameter_override_option @docker_common_options -@output_option +@structured_output_option @cli_framework_options @aws_creds_options @click.argument("resource_logical_id", required=False) @@ -240,7 +240,10 @@ def do_cli( # pylint: disable=too-many-locals, too-many-statements Implementation of the ``cli`` method """ + import json + from samcli.commands.build.build_context import BuildContext + from samcli.commands.build.exceptions import InvalidBuildDirException LOG.debug("'build' command is called") if cached: @@ -251,35 +254,51 @@ def do_cli( # pylint: disable=too-many-locals, too-many-statements processed_env_vars = process_env_var(container_env_var) processed_build_images = process_image_options(build_image) - with BuildContext( - function_identifier, - template, - base_dir, - build_dir, - cache_dir, - cached, - parallel=parallel, - clean=clean, - manifest_path=manifest_path, - use_container=use_container, - parameter_overrides=parameter_overrides, - docker_network=docker_network, - skip_pull_image=skip_pull_image, - mode=mode, - container_env_var=processed_env_vars, - container_env_var_file=container_env_var_file, - build_images=processed_build_images, - excluded_resources=exclude, - aws_region=click_ctx.region, - hook_name=hook_name, - build_in_source=build_in_source, - mount_with=mount_with, - mount_symlinks=mount_symlinks, - use_buildkit=use_buildkit, - language_extensions=language_extensions, - output=output, - ) as ctx: - ctx.run() + try: + with BuildContext( + function_identifier, + template, + base_dir, + build_dir, + cache_dir, + cached, + parallel=parallel, + clean=clean, + manifest_path=manifest_path, + use_container=use_container, + parameter_overrides=parameter_overrides, + docker_network=docker_network, + skip_pull_image=skip_pull_image, + mode=mode, + container_env_var=processed_env_vars, + container_env_var_file=container_env_var_file, + build_images=processed_build_images, + excluded_resources=exclude, + aws_region=click_ctx.region, + hook_name=hook_name, + build_in_source=build_in_source, + mount_with=mount_with, + mount_symlinks=mount_symlinks, + use_buildkit=use_buildkit, + language_extensions=language_extensions, + output=output, + ) as ctx: + ctx.run() + except InvalidBuildDirException as ex: + if output == "json": + click.echo( + json.dumps( + { + "status": "failure", + "error": { + "type": "InvalidBuildDirException", + "message": str(ex), + "resource": None, + }, + } + ) + ) + raise def _get_mode_value_from_envvar(name: str, choices: List[str]) -> Optional[str]: diff --git a/samcli/commands/build/utils.py b/samcli/commands/build/utils.py index 7f5668efbf3..5f496cf1c24 100644 --- a/samcli/commands/build/utils.py +++ b/samcli/commands/build/utils.py @@ -108,7 +108,8 @@ def prompt(config: CONFIG, source_dir: str) -> bool: f"mounting with write permissions to the source code directory {source_dir}. " f"Some files in this directory may be changed or added by the build process. " f"Pass `--mount-with WRITE` to `sam build` CLI to avoid this confirmation. " - f"\nWould you like to enable mounting with write permissions? " + f"\nWould you like to enable mounting with write permissions? ", + err=True, ): return True return False diff --git a/samcli/lib/build/build_strategy.py b/samcli/lib/build/build_strategy.py index ebe36d7acc8..fa5cfd96505 100644 --- a/samcli/lib/build/build_strategy.py +++ b/samcli/lib/build/build_strategy.py @@ -159,7 +159,7 @@ def build_single_function_definition(self, build_definition: FunctionBuildDefini UnsupportedBuilderLibraryVersionError, ) as ex: if getattr(ex, "resource_name", None) is None: - ex.resource_name = build_definition.get_full_path() # type: ignore[union-attr] + ex.resource_name = build_definition.get_full_path() raise def _do_build_single_function_definition(self, build_definition: FunctionBuildDefinition) -> Dict[str, str]: @@ -241,7 +241,7 @@ def build_single_layer_definition(self, layer_definition: LayerBuildDefinition) UnsupportedBuilderLibraryVersionError, ) as ex: if getattr(ex, "resource_name", None) is None: - ex.resource_name = layer_definition.full_path # type: ignore[union-attr] + ex.resource_name = layer_definition.full_path raise def _do_build_single_layer_definition(self, layer_definition: LayerBuildDefinition) -> Dict[str, str]: diff --git a/samcli/lib/build/exceptions.py b/samcli/lib/build/exceptions.py index e1f4ff50fcc..75621c2cd70 100644 --- a/samcli/lib/build/exceptions.py +++ b/samcli/lib/build/exceptions.py @@ -2,10 +2,14 @@ Build Related Exceptions. """ +from typing import Optional + from samcli.commands.exceptions import UserException class UnsupportedBuilderLibraryVersionError(Exception): + resource_name: Optional[str] = None + def __init__(self, container_name: str, error_msg: str) -> None: msg = ( "You are running an outdated version of Docker container '{container_name}' that is not compatible with" @@ -15,13 +19,15 @@ def __init__(self, container_name: str, error_msg: str) -> None: class BuildError(Exception): + resource_name: Optional[str] = None + def __init__(self, wrapped_from: str, msg: str) -> None: self.wrapped_from = wrapped_from Exception.__init__(self, msg) class BuildInsideContainerError(Exception): - pass + resource_name: Optional[str] = None class DockerConnectionError(BuildError): diff --git a/samcli/lib/build/workflow_config.py b/samcli/lib/build/workflow_config.py index 0a453880a17..5bbccde307b 100644 --- a/samcli/lib/build/workflow_config.py +++ b/samcli/lib/build/workflow_config.py @@ -27,7 +27,7 @@ class UnsupportedRuntimeException(Exception): - pass + resource_name: Optional[str] = None class UnsupportedBuilderException(Exception): diff --git a/tests/unit/commands/_utils/test_experimental.py b/tests/unit/commands/_utils/test_experimental.py index a73760a2806..2e202dd95b5 100644 --- a/tests/unit/commands/_utils/test_experimental.py +++ b/tests/unit/commands/_utils/test_experimental.py @@ -120,5 +120,5 @@ def test_prompt_experimental(self, update_experimental_context, enabled_mock, co prompt_experimental(config_entry, prompt) set_experimental_mock.assert_called_once_with(config_entry=config_entry, enabled=True) enabled_mock.assert_called_once_with(config_entry) - confirm_mock.assert_called_once_with(Colored().yellow(prompt), default=False) + confirm_mock.assert_called_once_with(Colored().yellow(prompt), default=False, err=True) update_experimental_context.assert_called_once() diff --git a/tests/unit/commands/buildcmd/test_build_context.py b/tests/unit/commands/buildcmd/test_build_context.py index 125e4f7b249..4a446a4ea29 100644 --- a/tests/unit/commands/buildcmd/test_build_context.py +++ b/tests/unit/commands/buildcmd/test_build_context.py @@ -1,5 +1,7 @@ +import io import json import os +from contextlib import redirect_stdout from unittest import TestCase from unittest.mock import ANY, MagicMock, Mock, call, patch @@ -1089,6 +1091,50 @@ def test_run_build_context( # assert that nested stack manager is called by both root stack and child stack given_nested_stack_manager.generate_auto_dependency_layer_stack.assert_has_calls([call(), call()]) + @patch("samcli.commands.build.build_context.SamLocalStackProvider.find_root_stack") + @patch("samcli.commands.build.build_context.BuildContext.set_up") + @patch("samcli.commands.build.build_context.BuildContext._handle_build_post_processing") + @patch("samcli.commands.build.build_context.BuildContext._handle_build_pre_processing") + @patch("samcli.commands.build.build_context.BuildContext.get_resources_to_build") + @patch("samcli.commands.build.build_context.BuildContext._is_sam_template", return_value=False) + @patch("samcli.commands.build.build_context.ApplicationBuilder") + def test_run_json_mode_emits_single_parseable_document( + self, + ApplicationBuilderMock, + is_sam_template_mock, + resources_mock, + pre_processing_mock, + post_processing_mock, + set_up_mock, + find_root_stack_mock, + ): + """run() in JSON mode must write exactly one parseable JSON document to stdout.""" + resources_mock.return_value = Mock(functions=[get_function("Fn", runtime="python3.12")], layers=[]) + ApplicationBuilderMock.return_value.build.return_value = ApplicationBuildResult(Mock(), "artifacts") + find_root_stack_mock.return_value.get_output_template_path.return_value = "build_dir/template.yaml" + + build_context = BuildContext( + resource_identifier=None, + template_file="template_file", + base_dir="base_dir", + build_dir="build_dir", + cache_dir="cache_dir", + cached=False, + parallel=False, + mode=None, + output="json", + ) + + captured = io.StringIO() + with redirect_stdout(captured): + build_context.run() + + # Must be exactly one JSON document on stdout -- json.loads fails on any extra output + result = json.loads(captured.getvalue()) + self.assertEqual(result["status"], "success") + self.assertIn("build_dir", result) + self.assertIn("resources", result) + @parameterized.expand( [ (UnsupportedRuntimeException(), "UnsupportedRuntimeException"), @@ -1464,29 +1510,6 @@ def test_json_with_functions_and_layers(self, echo_mock, secho_mock): self.assertEqual(result["resources"][1]["resource_id"], "Lyr") self.assertEqual(result["resources"][1]["compatible_runtimes"], ["python3.12", "python3.11"]) - @patch("samcli.commands.build.build_context.click.echo") - def test_json_functions_only(self, echo_mock): - collector = self._collector(functions=[get_function("Fn", runtime="python3.12")]) - - self.build_context._print_build_success("artifacts", "out_template", collector) - - result = json.loads(echo_mock.call_args[0][0]) - self.assertEqual(len(result["resources"]), 1) - self.assertEqual(result["resources"][0]["type"], "function") - - @patch("samcli.commands.build.build_context.click.echo") - def test_json_layers_only(self, echo_mock): - layer = DummyLayer("Lyr", "python3.12") - layer.full_path = "Lyr" - layer.compatible_runtimes = ["python3.12"] - collector = self._collector(layers=[layer]) - - self.build_context._print_build_success("artifacts", "out_template", collector) - - result = json.loads(echo_mock.call_args[0][0]) - self.assertEqual(len(result["resources"]), 1) - self.assertEqual(result["resources"][0]["type"], "layer") - @patch("samcli.commands.build.build_context.click.echo") def test_json_empty_collector(self, echo_mock): self.build_context._print_build_success("artifacts", "out_template", self._collector()) @@ -1495,19 +1518,20 @@ def test_json_empty_collector(self, echo_mock): self.assertEqual(result["resources"], []) @patch("samcli.commands.build.build_context.click.echo") - def test_json_architecture_defaults_to_none_when_absent(self, echo_mock): + def test_json_architecture_defaults_to_x86_64_when_absent(self, echo_mock): # get_function() builds a Function with architectures=None + # function.architecture resolves to X86_64 when absent collector = self._collector(functions=[get_function("Fn", runtime="python3.12")]) self.build_context._print_build_success("artifacts", "out_template", collector) result = json.loads(echo_mock.call_args[0][0]) - self.assertIsNone(result["resources"][0]["architecture"]) + self.assertEqual(result["resources"][0]["architecture"], "x86_64") @patch("samcli.commands.build.build_context.click.echo") - def test_json_multi_architecture_reports_first(self, echo_mock): - # Function is an immutable namedtuple, so build a variant with _replace - function = get_function("Fn", runtime="python3.12")._replace(architectures=["arm64", "x86_64"]) + def test_json_architecture_reports_specified_value(self, echo_mock): + # Function with explicit architecture + function = get_function("Fn", runtime="python3.12")._replace(architectures=["arm64"]) collector = self._collector(functions=[function]) self.build_context._print_build_success("artifacts", "out_template", collector) @@ -1515,22 +1539,6 @@ def test_json_multi_architecture_reports_first(self, echo_mock): result = json.loads(echo_mock.call_args[0][0]) self.assertEqual(result["resources"][0]["architecture"], "arm64") - @patch("samcli.commands.build.build_context.click.echo") - def test_json_emitted_even_when_success_message_disabled(self, echo_mock): - self.build_context._print_success_message = False - collector = self._collector(functions=[get_function("Fn", runtime="python3.12")]) - - self.build_context._print_build_success("artifacts", "out_template", collector) - - result = json.loads(echo_mock.call_args[0][0]) - self.assertEqual(result["status"], "success") - - @patch("samcli.commands.build.build_context.click.echo") - def test_json_uses_indentation(self, echo_mock): - self.build_context._print_build_success("artifacts", "out_template", self._collector()) - - self.assertIn('\n "status"', echo_mock.call_args[0][0]) - @patch("samcli.commands.build.build_context.click.secho") @patch("samcli.commands.build.build_context.click.echo") def test_text_mode_prints_banner_and_message(self, echo_mock, secho_mock): @@ -1599,20 +1607,10 @@ def test_json_resource_is_null_when_unattributable(self, echo_mock): self.build_context._print_build_failure(ex, "ResourceNotFound") error = json.loads(echo_mock.call_args[0][0])["error"] - # The key is always present so consumers can rely on the schema + # The key is emitted as null rather than omitted for unattributable failures self.assertIn("resource", error) self.assertIsNone(error["resource"]) - @patch("samcli.commands.build.build_context.click.echo") - def test_json_exception_resource_name_wins_over_identifier(self, echo_mock): - ex = BuildError(wrapped_from="BuildError", msg="msg") - ex.resource_name = "FromException" - - self.build_context._print_build_failure(ex, "BuildError") - - error = json.loads(echo_mock.call_args[0][0])["error"] - self.assertEqual(error["resource"], "FromException") - @patch("samcli.commands.build.build_context.click.echo") def test_json_error_shape(self, echo_mock): ex = BuildError(wrapped_from="BuildError", msg="something broke") @@ -1641,14 +1639,6 @@ def test_json_error_type_passed_through(self, error_type, echo_mock): result = json.loads(echo_mock.call_args[0][0]) self.assertEqual(result["error"]["type"], error_type) - @patch("samcli.commands.build.build_context.click.secho") - @patch("samcli.commands.build.build_context.click.echo") - def test_json_emitted_even_when_text_banner_suppressed(self, echo_mock, secho_mock): - self.build_context._print_build_failure(FunctionNotFound("nope"), "FunctionNotFound", print_text_banner=False) - - self.assertEqual(echo_mock.call_count, 1) - secho_mock.assert_not_called() - @patch("samcli.commands.build.build_context.click.secho") @patch("samcli.commands.build.build_context.click.echo") def test_text_mode_prints_banner(self, echo_mock, secho_mock): diff --git a/tests/unit/commands/buildcmd/test_command.py b/tests/unit/commands/buildcmd/test_command.py index babc5a58bc6..020aab21a39 100644 --- a/tests/unit/commands/buildcmd/test_command.py +++ b/tests/unit/commands/buildcmd/test_command.py @@ -1,3 +1,4 @@ +import json import os import click @@ -5,6 +6,7 @@ from unittest.mock import Mock, patch from samcli.commands.build.command import do_cli, _get_mode_value_from_envvar +from samcli.commands.build.exceptions import InvalidBuildDirException from samcli.commands.build.utils import MountMode From c33c005be842f4de6d97c3969f7915e2b845d8f6 Mon Sep 17 00:00:00 2001 From: Madhav Donthula Date: Wed, 5 Aug 2026 16:09:14 -0700 Subject: [PATCH 11/22] fix: centralize JSON failure serialization in do_cli Move all --output json failure serialization to a single except UserException handler in do_cli, covering every failure path uniformly (missing layer BuildMethod, invalid build dir, pre-processing errors) instead of only the exception types caught inside run(). - Add build_failure_json() helper in build_context as the single source of truth for the failure wire format - do_cli uses the helper and checks OutputOption(output) is OutputOption.json (enum, not raw string literal) - run() only prints the text "Build Failed" banner and re-raises UserException carrying resource_name; no double-emit - Add resource_name class attribute to UserException - Move JSON failure tests to test_command.py (do_cli level); simplify _print_build_failure tests to text-banner only --- samcli/commands/build/build_context.py | 62 +++++++++------- samcli/commands/build/command.py | 29 +++----- samcli/commands/exceptions.py | 5 ++ .../commands/buildcmd/test_build_context.py | 74 +++---------------- tests/unit/commands/buildcmd/test_command.py | 60 ++++++++++++++- 5 files changed, 121 insertions(+), 109 deletions(-) diff --git a/samcli/commands/build/build_context.py b/samcli/commands/build/build_context.py index 0d273f58d09..237b1b25ee1 100644 --- a/samcli/commands/build/build_context.py +++ b/samcli/commands/build/build_context.py @@ -71,6 +71,25 @@ LOG = logging.getLogger(__name__) +def build_failure_json(ex: Exception) -> str: + """Serialize a build failure into the structured JSON error document. + + Single source of truth for the failure wire format, shared by do_cli (which handles + every failure path) and any other caller, so the schema lives in exactly one place. + """ + error_type = getattr(ex, "wrapped_from", None) or type(ex).__name__ + return json.dumps( + { + "status": "failure", + "error": { + "type": error_type, + "message": str(ex), + "resource": getattr(ex, "resource_name", None), + }, + } + ) + + class BuildContext: def __init__( self, @@ -348,10 +367,10 @@ def run(self) -> None: except FunctionNotFound as function_not_found_ex: caught_exception = function_not_found_ex - self._print_build_failure(function_not_found_ex, "FunctionNotFound", print_text_banner=False) - raise UserException( - str(function_not_found_ex), wrapped_from=function_not_found_ex.__class__.__name__ - ) from function_not_found_ex + self._print_build_failure(print_text_banner=False) + user_ex = UserException(str(function_not_found_ex), wrapped_from=function_not_found_ex.__class__.__name__) + user_ex.resource_name = getattr(function_not_found_ex, "resource_name", None) + raise user_ex from function_not_found_ex except ( UnsupportedRuntimeException, BuildError, @@ -367,9 +386,11 @@ def run(self) -> None: deep_wrap = getattr(ex, "wrapped_from", None) wrapped_from = deep_wrap if deep_wrap else ex.__class__.__name__ - self._print_build_failure(ex, wrapped_from) + self._print_build_failure() - raise UserException(str(ex), wrapped_from=wrapped_from) from ex + user_ex = UserException(str(ex), wrapped_from=wrapped_from) + user_ex.resource_name = getattr(ex, "resource_name", None) + raise user_ex from ex finally: if self.build_in_source: exception_name = type(caught_exception).__name__ if caught_exception else None @@ -1132,34 +1153,21 @@ def _print_build_success( ) click.secho(msg, fg="yellow") - def _print_build_failure(self, ex: Exception, error_type: str, print_text_banner: bool = True) -> None: + def _print_build_failure(self, print_text_banner: bool = True) -> None: """ - Reports a failed build, either as structured JSON or as a human readable message. + Prints the human-readable "Build Failed" banner in text mode. + + JSON-mode failure serialization is handled centrally in do_cli so that a single + handler covers every failure path (including exceptions raised before run()'s + try block, e.g. template parse errors or missing layer BuildMethod). Parameters ---------- - ex: Exception - The exception that caused the build to fail - error_type: str - The error type to report print_text_banner: bool Whether to print the "Build Failed" banner in text mode """ - if self._output is not OutputOption.json: - if print_text_banner: - click.secho("\nBuild Failed", fg="red") - return - - # The resource key is included in the error JSON when available. It is - # populated from the exception when the failure is attributable to a specific - # resource, or from the resource the customer asked to build. It may be - # None when the failure is not tied to any single resource (e.g. invalid - # build directory, template parse errors). - resource_name = getattr(ex, "resource_name", None) or self._resource_identifier - - error: Dict[str, Any] = {"type": error_type, "message": str(ex), "resource": resource_name} - - click.echo(json.dumps({"status": "failure", "error": error})) + if self._output is not OutputOption.json and print_text_banner: + click.secho("\nBuild Failed", fg="red") def _gen_success_msg(self, artifacts_dir: str, output_template_path: str, is_default_build_dir: bool) -> str: """ diff --git a/samcli/commands/build/command.py b/samcli/commands/build/command.py index 47dfea3d11c..f0d828d4ee9 100644 --- a/samcli/commands/build/command.py +++ b/samcli/commands/build/command.py @@ -240,10 +240,9 @@ def do_cli( # pylint: disable=too-many-locals, too-many-statements Implementation of the ``cli`` method """ - import json - - from samcli.commands.build.build_context import BuildContext - from samcli.commands.build.exceptions import InvalidBuildDirException + from samcli.commands.build.build_context import BuildContext, build_failure_json + from samcli.commands.exceptions import UserException + from samcli.lib.observability.util import OutputOption LOG.debug("'build' command is called") if cached: @@ -284,20 +283,14 @@ def do_cli( # pylint: disable=too-many-locals, too-many-statements output=output, ) as ctx: ctx.run() - except InvalidBuildDirException as ex: - if output == "json": - click.echo( - json.dumps( - { - "status": "failure", - "error": { - "type": "InvalidBuildDirException", - "message": str(ex), - "resource": None, - }, - } - ) - ) + except UserException as ex: + # Central JSON failure serialization for sam build --output json. Catching UserException + # here (rather than per-exception inside run()) covers every failure path uniformly, + # including exceptions raised before run()'s try block (missing layer BuildMethod) and + # the __enter__/set_up phase (invalid build dir). run() only prints the text-mode + # "Build Failed" banner and re-raises, so there is no double-emit. + if OutputOption(output) is OutputOption.json: + click.echo(build_failure_json(ex)) raise diff --git a/samcli/commands/exceptions.py b/samcli/commands/exceptions.py index 5f835e0d33d..30c1604d9c8 100644 --- a/samcli/commands/exceptions.py +++ b/samcli/commands/exceptions.py @@ -23,6 +23,11 @@ class UserException(click.ClickException): exit_code = 1 + # Optional logical ID of the resource a failure is attributable to. Used by commands + # that emit structured (JSON) error output. Defaults to None for failures not tied to + # a single resource. + resource_name: Optional[str] = None + def __init__(self, message, wrapped_from=None): self.wrapped_from = wrapped_from diff --git a/tests/unit/commands/buildcmd/test_build_context.py b/tests/unit/commands/buildcmd/test_build_context.py index 4a446a4ea29..10291e558a0 100644 --- a/tests/unit/commands/buildcmd/test_build_context.py +++ b/tests/unit/commands/buildcmd/test_build_context.py @@ -1578,73 +1578,12 @@ def setUp(self): output="json", ) - @patch("samcli.commands.build.build_context.click.echo") - def test_json_resource_from_exception(self, echo_mock): - ex = BuildError(wrapped_from="WorkflowFailedError", msg="dependency failure") - ex.resource_name = "HelloWorldFunction" - - self.build_context._print_build_failure(ex, "WorkflowFailedError") - - error = json.loads(echo_mock.call_args[0][0])["error"] - self.assertEqual(error["resource"], "HelloWorldFunction") - - @patch("samcli.commands.build.build_context.click.echo") - def test_json_resource_falls_back_to_resource_identifier(self, echo_mock): - # ResourceNotFound carries no resource_name, so the resource the user asked - # to build is used - even though it does not exist in the template. - ex = ResourceNotFound("Unable to find a function or layer with name 'function_identifier'") - - self.build_context._print_build_failure(ex, "ResourceNotFound") - - error = json.loads(echo_mock.call_args[0][0])["error"] - self.assertEqual(error["resource"], "function_identifier") - - @patch("samcli.commands.build.build_context.click.echo") - def test_json_resource_is_null_when_unattributable(self, echo_mock): - self.build_context._resource_identifier = None - ex = ResourceNotFound("no resource") - - self.build_context._print_build_failure(ex, "ResourceNotFound") - - error = json.loads(echo_mock.call_args[0][0])["error"] - # The key is emitted as null rather than omitted for unattributable failures - self.assertIn("resource", error) - self.assertIsNone(error["resource"]) - - @patch("samcli.commands.build.build_context.click.echo") - def test_json_error_shape(self, echo_mock): - ex = BuildError(wrapped_from="BuildError", msg="something broke") - - self.build_context._print_build_failure(ex, "BuildError") - - result = json.loads(echo_mock.call_args[0][0]) - self.assertEqual(result["status"], "failure") - self.assertEqual(set(result["error"].keys()), {"type", "message", "resource"}) - self.assertEqual(result["error"]["type"], "BuildError") - self.assertEqual(result["error"]["message"], "something broke") - - @parameterized.expand( - [ - ("FunctionNotFound",), - ("UnsupportedRuntimeException",), - ("BuildInsideContainerError",), - ("InvalidBuildGraphException",), - ("ResourceNotFound",), - ] - ) - @patch("samcli.commands.build.build_context.click.echo") - def test_json_error_type_passed_through(self, error_type, echo_mock): - self.build_context._print_build_failure(BuildError("wrap", "msg"), error_type) - - result = json.loads(echo_mock.call_args[0][0]) - self.assertEqual(result["error"]["type"], error_type) - @patch("samcli.commands.build.build_context.click.secho") @patch("samcli.commands.build.build_context.click.echo") def test_text_mode_prints_banner(self, echo_mock, secho_mock): self.build_context._output = "text" - self.build_context._print_build_failure(BuildError("wrap", "msg"), "BuildError") + self.build_context._print_build_failure() echo_mock.assert_not_called() secho_mock.assert_called_once_with("\nBuild Failed", fg="red") @@ -1654,7 +1593,16 @@ def test_text_mode_prints_banner(self, echo_mock, secho_mock): def test_text_mode_prints_nothing_when_banner_suppressed(self, echo_mock, secho_mock): self.build_context._output = "text" - self.build_context._print_build_failure(FunctionNotFound("nope"), "FunctionNotFound", print_text_banner=False) + self.build_context._print_build_failure(print_text_banner=False) + + echo_mock.assert_not_called() + secho_mock.assert_not_called() + + @patch("samcli.commands.build.build_context.click.secho") + @patch("samcli.commands.build.build_context.click.echo") + def test_json_mode_emits_no_text_banner(self, echo_mock, secho_mock): + # In JSON mode, _print_build_failure stays silent; do_cli serializes the failure. + self.build_context._print_build_failure() echo_mock.assert_not_called() secho_mock.assert_not_called() diff --git a/tests/unit/commands/buildcmd/test_command.py b/tests/unit/commands/buildcmd/test_command.py index 020aab21a39..40f70490043 100644 --- a/tests/unit/commands/buildcmd/test_command.py +++ b/tests/unit/commands/buildcmd/test_command.py @@ -6,8 +6,9 @@ from unittest.mock import Mock, patch from samcli.commands.build.command import do_cli, _get_mode_value_from_envvar -from samcli.commands.build.exceptions import InvalidBuildDirException +from samcli.commands.build.exceptions import MissingBuildMethodException from samcli.commands.build.utils import MountMode +from samcli.commands.exceptions import UserException class TestDoCli(TestCase): @@ -78,6 +79,63 @@ def test_must_succeed_build(self, os_mock, BuildContextMock, mock_build_click): self.assertEqual(ctx_mock.run.call_count, 1) +class TestDoCliJsonFailure(TestCase): + """do_cli centrally serializes failures to JSON in --output json mode, covering every + UserException path (including non-BuildError ones the earlier per-exception handling missed).""" + + def _run_expecting(self, raised_exception): + base_args = ["function_identifier", "template", "base_dir", "build_dir", "cache_dir", "clean"] + echoed = [] + with patch("samcli.commands.build.build_context.BuildContext") as BuildContextMock: + BuildContextMock.return_value.__enter__.return_value.run.side_effect = raised_exception + with patch("samcli.commands.build.command.click.echo", side_effect=echoed.append): + with self.assertRaises(type(raised_exception)): + do_cli( + Mock(), + *base_args, + "use_container", + "cached", + "parallel", + "manifest_path", + "docker_network", + "skip_pull_image", + "parameter_overrides", + "mode", + (""), + "container_env_var_file", + (), + (), + hook_name=None, + build_in_source=False, + mount_with=MountMode.READ, + mount_symlinks=True, + use_buildkit=False, + language_extensions=None, + output="json", + ) + return echoed + + def test_json_failure_includes_type_message_and_resource(self): + # run() re-raises build failures as UserException, carrying wrapped_from + resource_name. + ex = UserException("dependency failure", wrapped_from="WorkflowFailedError") + ex.resource_name = "HelloWorldFunction" + + result = json.loads(self._run_expecting(ex)[0]) + + self.assertEqual(result["status"], "failure") + self.assertEqual(result["error"]["type"], "WorkflowFailedError") + self.assertEqual(result["error"]["message"], "dependency failure") + self.assertEqual(result["error"]["resource"], "HelloWorldFunction") + + def test_json_failure_for_non_build_error_user_exception(self): + # MissingBuildMethodException is a UserException raised before/around run()'s try block. + # Previously it exited 1 with empty stdout; do_cli's UserException handler must emit JSON. + result = json.loads(self._run_expecting(MissingBuildMethodException("no build method"))[0]) + + self.assertEqual(result["status"], "failure") + self.assertEqual(result["error"]["type"], "MissingBuildMethodException") + + class TestGetModeValueFromEnvvar(TestCase): def setUp(self): self.original = os.environ.copy() From 9a951d297b7476a61e901f85cb33de0ef0052b00 Mon Sep 17 00:00:00 2001 From: Madhav Donthula Date: Wed, 5 Aug 2026 16:16:54 -0700 Subject: [PATCH 12/22] test: consolidate and harden _print_build_failure tests - Merge two near-identical "stays silent" tests into one parameterized test - Use OutputOption enum instead of raw "text"/"json" strings so the tests match how the real code sets _output (via enum conversion in __init__), removing a latent fragility where raw strings passed only by coincidence --- .../commands/buildcmd/test_build_context.py | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/tests/unit/commands/buildcmd/test_build_context.py b/tests/unit/commands/buildcmd/test_build_context.py index 10291e558a0..3ea58522812 100644 --- a/tests/unit/commands/buildcmd/test_build_context.py +++ b/tests/unit/commands/buildcmd/test_build_context.py @@ -20,6 +20,7 @@ from samcli.lib.build.build_graph import DEFAULT_DEPENDENCIES_DIR from samcli.lib.build.bundler import EsbuildBundlerManager from samcli.lib.build.workflow_config import UnsupportedRuntimeException +from samcli.lib.observability.util import OutputOption from samcli.lib.providers.provider import Function, ResourcesToBuildCollector, get_function_build_info from samcli.lib.telemetry.event import EventName, UsedFeature from samcli.lib.utils.osutils import BUILD_DIR_PERMISSIONS @@ -1581,29 +1582,27 @@ def setUp(self): @patch("samcli.commands.build.build_context.click.secho") @patch("samcli.commands.build.build_context.click.echo") def test_text_mode_prints_banner(self, echo_mock, secho_mock): - self.build_context._output = "text" + self.build_context._output = OutputOption.text self.build_context._print_build_failure() echo_mock.assert_not_called() secho_mock.assert_called_once_with("\nBuild Failed", fg="red") + @parameterized.expand( + [ + (OutputOption.text, False), # text mode, banner explicitly suppressed + (OutputOption.json, True), # JSON mode: do_cli serializes, this method stays silent + ] + ) @patch("samcli.commands.build.build_context.click.secho") @patch("samcli.commands.build.build_context.click.echo") - def test_text_mode_prints_nothing_when_banner_suppressed(self, echo_mock, secho_mock): - self.build_context._output = "text" - - self.build_context._print_build_failure(print_text_banner=False) + def test_print_build_failure_stays_silent(self, output, print_banner, echo_mock, secho_mock): + self.build_context._output = output - echo_mock.assert_not_called() - secho_mock.assert_not_called() - - @patch("samcli.commands.build.build_context.click.secho") - @patch("samcli.commands.build.build_context.click.echo") - def test_json_mode_emits_no_text_banner(self, echo_mock, secho_mock): - # In JSON mode, _print_build_failure stays silent; do_cli serializes the failure. - self.build_context._print_build_failure() + self.build_context._print_build_failure(print_text_banner=print_banner) + # Never emits JSON (that is do_cli's job); prints no banner in these two cases echo_mock.assert_not_called() secho_mock.assert_not_called() From 588ff68a3669c123abb97edbbf0d357c5333a29c Mon Sep 17 00:00:00 2001 From: Madhav Donthula Date: Wed, 5 Aug 2026 16:34:10 -0700 Subject: [PATCH 13/22] fix: route UnsupportedBuilderException through JSON failure handler UnsupportedBuilderException (raised for an invalid Metadata.BuildMethod, e.g. a template typo) subclasses plain Exception, so it escaped both run()'s except tuple and do_cli's UserException handler - exiting 1 with empty stdout in --output json. - Add it to run()'s except tuple so it converts to UserException and routes through build_failure_json, matching its sibling UnsupportedRuntimeException - Add it to the DefaultBuildStrategy except tuples so resource_name is attributed - Add resource_name class attribute (Comment #5 pattern) - Extend test_must_catch_known_exceptions to cover it --- samcli/commands/build/build_context.py | 3 ++- samcli/lib/build/build_strategy.py | 4 +++- samcli/lib/build/workflow_config.py | 2 +- tests/unit/commands/buildcmd/test_build_context.py | 3 ++- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/samcli/commands/build/build_context.py b/samcli/commands/build/build_context.py index 237b1b25ee1..27a10a0560c 100644 --- a/samcli/commands/build/build_context.py +++ b/samcli/commands/build/build_context.py @@ -37,7 +37,7 @@ BuildInsideContainerError, InvalidBuildGraphException, ) -from samcli.lib.build.workflow_config import UnsupportedRuntimeException +from samcli.lib.build.workflow_config import UnsupportedBuilderException, UnsupportedRuntimeException from samcli.lib.cfn_language_extensions.models import PACKAGEABLE_RESOURCE_ARTIFACT_PROPERTIES from samcli.lib.cfn_language_extensions.sam_integration import ( contains_loop_variable, @@ -373,6 +373,7 @@ def run(self) -> None: raise user_ex from function_not_found_ex except ( UnsupportedRuntimeException, + UnsupportedBuilderException, BuildError, BuildInsideContainerError, UnsupportedBuilderLibraryVersionError, diff --git a/samcli/lib/build/build_strategy.py b/samcli/lib/build/build_strategy.py index fa5cfd96505..bda16c8faa6 100644 --- a/samcli/lib/build/build_strategy.py +++ b/samcli/lib/build/build_strategy.py @@ -27,7 +27,7 @@ UnsupportedBuilderLibraryVersionError, ) from samcli.lib.build.utils import warn_on_invalid_architecture -from samcli.lib.build.workflow_config import UnsupportedRuntimeException +from samcli.lib.build.workflow_config import UnsupportedBuilderException, UnsupportedRuntimeException from samcli.lib.utils import osutils from samcli.lib.utils.architecture import X86_64 from samcli.lib.utils.async_utils import AsyncContext @@ -155,6 +155,7 @@ def build_single_function_definition(self, build_definition: FunctionBuildDefini except ( BuildError, UnsupportedRuntimeException, + UnsupportedBuilderException, BuildInsideContainerError, UnsupportedBuilderLibraryVersionError, ) as ex: @@ -237,6 +238,7 @@ def build_single_layer_definition(self, layer_definition: LayerBuildDefinition) except ( BuildError, UnsupportedRuntimeException, + UnsupportedBuilderException, BuildInsideContainerError, UnsupportedBuilderLibraryVersionError, ) as ex: diff --git a/samcli/lib/build/workflow_config.py b/samcli/lib/build/workflow_config.py index 5bbccde307b..a4f97f955a9 100644 --- a/samcli/lib/build/workflow_config.py +++ b/samcli/lib/build/workflow_config.py @@ -31,7 +31,7 @@ class UnsupportedRuntimeException(Exception): class UnsupportedBuilderException(Exception): - pass + resource_name: Optional[str] = None WorkFlowSelector = Union["BasicWorkflowSelector", "ManifestWorkflowSelector"] diff --git a/tests/unit/commands/buildcmd/test_build_context.py b/tests/unit/commands/buildcmd/test_build_context.py index 3ea58522812..2a54345cba5 100644 --- a/tests/unit/commands/buildcmd/test_build_context.py +++ b/tests/unit/commands/buildcmd/test_build_context.py @@ -19,7 +19,7 @@ ) from samcli.lib.build.build_graph import DEFAULT_DEPENDENCIES_DIR from samcli.lib.build.bundler import EsbuildBundlerManager -from samcli.lib.build.workflow_config import UnsupportedRuntimeException +from samcli.lib.build.workflow_config import UnsupportedBuilderException, UnsupportedRuntimeException from samcli.lib.observability.util import OutputOption from samcli.lib.providers.provider import Function, ResourcesToBuildCollector, get_function_build_info from samcli.lib.telemetry.event import EventName, UsedFeature @@ -1139,6 +1139,7 @@ def test_run_json_mode_emits_single_parseable_document( @parameterized.expand( [ (UnsupportedRuntimeException(), "UnsupportedRuntimeException"), + (UnsupportedBuilderException(), "UnsupportedBuilderException"), (BuildInsideContainerError(), "BuildInsideContainerError"), (BuildError(wrapped_from=DeepWrap().__class__.__name__, msg="Test"), "DeepWrap"), ( From 985e7a45c38c016bce2249090b59d350dcd2538a Mon Sep 17 00:00:00 2001 From: Madhav Donthula Date: Wed, 5 Aug 2026 16:49:24 -0700 Subject: [PATCH 14/22] fix: attribute resource on FunctionNotFound and MissingBuildMethod failures Failure paths that know exactly which resource failed were reporting "resource": null in --output json: - FunctionNotFound handler had a dead _print_build_failure(print_text_banner=False) no-op and a getattr that FunctionNotFound never carries. Both failure handlers now fall back to self._resource_identifier, so sam build reports that name. - MissingBuildMethodException (raised from _collect_single_buildable_layer, a UserException) was not in run()'s except tuple, so it reached do_cli with no resource attribution. Added it; the layer name now populates via the fallback. Tests: use OutputOption.text (not the raw string) in the two success-path text-mode tests so they actually exercise the enum comparison, and assert resource_name fallback in test_must_catch_known_exceptions. --- samcli/commands/build/build_context.py | 9 ++++++--- tests/unit/commands/buildcmd/test_build_context.py | 7 +++++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/samcli/commands/build/build_context.py b/samcli/commands/build/build_context.py index 27a10a0560c..f4b6b02c4b6 100644 --- a/samcli/commands/build/build_context.py +++ b/samcli/commands/build/build_context.py @@ -367,9 +367,9 @@ def run(self) -> None: except FunctionNotFound as function_not_found_ex: caught_exception = function_not_found_ex - self._print_build_failure(print_text_banner=False) user_ex = UserException(str(function_not_found_ex), wrapped_from=function_not_found_ex.__class__.__name__) - user_ex.resource_name = getattr(function_not_found_ex, "resource_name", None) + # The failure is attributable to the specific resource the user asked to build. + user_ex.resource_name = getattr(function_not_found_ex, "resource_name", None) or self._resource_identifier raise user_ex from function_not_found_ex except ( UnsupportedRuntimeException, @@ -378,6 +378,7 @@ def run(self) -> None: BuildInsideContainerError, UnsupportedBuilderLibraryVersionError, InvalidBuildGraphException, + MissingBuildMethodException, ResourceNotFound, ) as ex: caught_exception = ex @@ -390,7 +391,9 @@ def run(self) -> None: self._print_build_failure() user_ex = UserException(str(ex), wrapped_from=wrapped_from) - user_ex.resource_name = getattr(ex, "resource_name", None) + # Prefer the resource the exception attributes the failure to; otherwise fall back to + # the resource the user asked to build (relevant even when it doesn't exist). + user_ex.resource_name = getattr(ex, "resource_name", None) or self._resource_identifier raise user_ex from ex finally: if self.build_in_source: diff --git a/tests/unit/commands/buildcmd/test_build_context.py b/tests/unit/commands/buildcmd/test_build_context.py index 2a54345cba5..8a5069f4529 100644 --- a/tests/unit/commands/buildcmd/test_build_context.py +++ b/tests/unit/commands/buildcmd/test_build_context.py @@ -1228,6 +1228,9 @@ def test_must_catch_known_exceptions( self.assertEqual(str(ctx.exception), str(exception)) self.assertEqual(wrapped_exception, ctx.exception.wrapped_from) + # None of these exceptions carry resource_name, so the re-raised UserException falls + # back to the resource the user asked to build. + self.assertEqual(ctx.exception.resource_name, "function_identifier") @patch("samcli.commands.build.build_context.SamLocalStackProvider.get_stacks") @patch("samcli.commands.build.build_context.SamApiProvider") @@ -1544,7 +1547,7 @@ def test_json_architecture_reports_specified_value(self, echo_mock): @patch("samcli.commands.build.build_context.click.secho") @patch("samcli.commands.build.build_context.click.echo") def test_text_mode_prints_banner_and_message(self, echo_mock, secho_mock): - self.build_context._output = "text" + self.build_context._output = OutputOption.text collector = self._collector(functions=[get_function("Fn", runtime="python3.12")]) self.build_context._print_build_success("artifacts", "out_template", collector) @@ -1557,7 +1560,7 @@ def test_text_mode_prints_banner_and_message(self, echo_mock, secho_mock): @patch("samcli.commands.build.build_context.click.secho") @patch("samcli.commands.build.build_context.click.echo") def test_text_mode_banner_only_when_success_message_disabled(self, echo_mock, secho_mock): - self.build_context._output = "text" + self.build_context._output = OutputOption.text self.build_context._print_success_message = False self.build_context._print_build_success("artifacts", "out_template", self._collector()) From f46d027d2c8dde0951cfdb6454bffcbae03aedd9 Mon Sep 17 00:00:00 2001 From: Madhav Donthula Date: Wed, 5 Aug 2026 17:15:09 -0700 Subject: [PATCH 15/22] fix: wrap rmtree in _setup_build_dir; soften do_cli failure-coverage comment - Extend the try in _setup_build_dir to cover shutil.rmtree, not just mkdir. An OSError while clearing the build dir (permission denied, file held open on Windows) now surfaces as InvalidBuildDirException -> JSON error, instead of a bare OSError that exits 1 with empty stdout in --output json. - Soften the do_cli handler comment: it covers user-facing failures, not "every failure path"; note that an unexpected internal error (bare Exception) is not serialized. - Add test_rmtree_oserror_is_converted_to_invalid_build_dir. --- samcli/commands/build/build_context.py | 8 ++++---- samcli/commands/build/command.py | 10 ++++++---- .../unit/commands/buildcmd/test_build_context.py | 16 ++++++++++++++++ 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/samcli/commands/build/build_context.py b/samcli/commands/build/build_context.py index f4b6b02c4b6..8b0137d3709 100644 --- a/samcli/commands/build/build_context.py +++ b/samcli/commands/build/build_context.py @@ -1238,11 +1238,11 @@ def _setup_build_dir(build_dir: str, clean: bool) -> str: ) raise InvalidBuildDirException(exception_message) - if build_path.exists() and os.listdir(build_dir) and clean: - # build folder contains something inside. Clear everything. - shutil.rmtree(build_dir) - try: + if build_path.exists() and os.listdir(build_dir) and clean: + # build folder contains something inside. Clear everything. + shutil.rmtree(build_dir) + build_path.mkdir(mode=BUILD_DIR_PERMISSIONS, parents=True, exist_ok=True) except OSError as ex: raise InvalidBuildDirException(f"Unable to use build dir {build_dir}. Reason: {str(ex)}") from ex diff --git a/samcli/commands/build/command.py b/samcli/commands/build/command.py index f0d828d4ee9..35250bc8258 100644 --- a/samcli/commands/build/command.py +++ b/samcli/commands/build/command.py @@ -285,10 +285,12 @@ def do_cli( # pylint: disable=too-many-locals, too-many-statements ctx.run() except UserException as ex: # Central JSON failure serialization for sam build --output json. Catching UserException - # here (rather than per-exception inside run()) covers every failure path uniformly, - # including exceptions raised before run()'s try block (missing layer BuildMethod) and - # the __enter__/set_up phase (invalid build dir). run() only prints the text-mode - # "Build Failed" banner and re-raises, so there is no double-emit. + # here (rather than per-exception inside run()) covers all user-facing failures uniformly, + # including exceptions raised before run()'s try block (missing layer BuildMethod) and the + # __enter__/set_up phase (invalid build dir). run() only prints the text-mode "Build Failed" + # banner and re-raises, so there is no double-emit. + # Note: only UserException subclasses are serialized. An unexpected internal error (a bug + # surfacing as a bare Exception) is not converted to JSON and propagates as today. if OutputOption(output) is OutputOption.json: click.echo(build_failure_json(ex)) raise diff --git a/tests/unit/commands/buildcmd/test_build_context.py b/tests/unit/commands/buildcmd/test_build_context.py index 8a5069f4529..ee8cb19f686 100644 --- a/tests/unit/commands/buildcmd/test_build_context.py +++ b/tests/unit/commands/buildcmd/test_build_context.py @@ -771,6 +771,22 @@ def test_build_dir_exists_with_non_empty_dir(self, pathlib_patch, os_patch, shut shutil_patch.rmtree.assert_called_once_with(build_dir) pathlib_patch.Path.cwd.assert_called_once() + @patch("samcli.commands.build.build_context.shutil") + @patch("samcli.commands.build.build_context.os") + @patch("samcli.commands.build.build_context.pathlib") + def test_rmtree_oserror_is_converted_to_invalid_build_dir(self, pathlib_patch, os_patch, shutil_patch): + # A failure clearing the build dir (e.g. permission denied, file held open on Windows) + # must surface as InvalidBuildDirException so --output json still emits a JSON error. + path_mock = Mock() + pathlib_patch.Path.return_value = path_mock + os_patch.path.abspath.side_effect = ["/somepath", "/cwd/path"] + path_mock.exists.return_value = True + os_patch.listdir.return_value = True + shutil_patch.rmtree.side_effect = OSError("permission denied") + + with self.assertRaises(InvalidBuildDirException): + BuildContext._setup_build_dir("/somepath", True) + @patch("samcli.commands.build.build_context.shutil") @patch("samcli.commands.build.build_context.os") @patch("samcli.commands.build.build_context.pathlib") From 90dd7f6b5d63e7f3e09f8a84bcd5bc2229cdd20b Mon Sep 17 00:00:00 2001 From: Madhav Donthula Date: Wed, 5 Aug 2026 17:32:54 -0700 Subject: [PATCH 16/22] fix: broaden do_cli JSON failure handler to all exceptions; drop dead param - Change do_cli's except UserException to except Exception so every build-path failure emits a JSON document, including bare-Exception template errors (InvalidLayerReference, RemoteStackLocationNotSupported, MissingCodeUri, InvalidTemplateFile) that are not UserException subclasses. Re-raises unconditionally so @track_command still records telemetry and wraps non-UserException as UnhandledException. - Drop the now-dead print_text_banner parameter from _print_build_failure; no caller passes False since the FunctionNotFound handler stopped calling it. - Add test_json_failure_for_bare_exception; simplify the silence test. --- samcli/commands/build/build_context.py | 9 ++------- samcli/commands/build/command.py | 17 ++++++++--------- .../commands/buildcmd/test_build_context.py | 14 ++++---------- tests/unit/commands/buildcmd/test_command.py | 8 ++++++++ 4 files changed, 22 insertions(+), 26 deletions(-) diff --git a/samcli/commands/build/build_context.py b/samcli/commands/build/build_context.py index 8b0137d3709..dbeff3ea951 100644 --- a/samcli/commands/build/build_context.py +++ b/samcli/commands/build/build_context.py @@ -1157,20 +1157,15 @@ def _print_build_success( ) click.secho(msg, fg="yellow") - def _print_build_failure(self, print_text_banner: bool = True) -> None: + def _print_build_failure(self) -> None: """ Prints the human-readable "Build Failed" banner in text mode. JSON-mode failure serialization is handled centrally in do_cli so that a single handler covers every failure path (including exceptions raised before run()'s try block, e.g. template parse errors or missing layer BuildMethod). - - Parameters - ---------- - print_text_banner: bool - Whether to print the "Build Failed" banner in text mode """ - if self._output is not OutputOption.json and print_text_banner: + if self._output is not OutputOption.json: click.secho("\nBuild Failed", fg="red") def _gen_success_msg(self, artifacts_dir: str, output_template_path: str, is_default_build_dir: bool) -> str: diff --git a/samcli/commands/build/command.py b/samcli/commands/build/command.py index 35250bc8258..d0a33613686 100644 --- a/samcli/commands/build/command.py +++ b/samcli/commands/build/command.py @@ -241,7 +241,6 @@ def do_cli( # pylint: disable=too-many-locals, too-many-statements """ from samcli.commands.build.build_context import BuildContext, build_failure_json - from samcli.commands.exceptions import UserException from samcli.lib.observability.util import OutputOption LOG.debug("'build' command is called") @@ -283,14 +282,14 @@ def do_cli( # pylint: disable=too-many-locals, too-many-statements output=output, ) as ctx: ctx.run() - except UserException as ex: - # Central JSON failure serialization for sam build --output json. Catching UserException - # here (rather than per-exception inside run()) covers all user-facing failures uniformly, - # including exceptions raised before run()'s try block (missing layer BuildMethod) and the - # __enter__/set_up phase (invalid build dir). run() only prints the text-mode "Build Failed" - # banner and re-raises, so there is no double-emit. - # Note: only UserException subclasses are serialized. An unexpected internal error (a bug - # surfacing as a bare Exception) is not converted to JSON and propagates as today. + except Exception as ex: + # Central JSON failure serialization for sam build --output json. Catching broadly here + # (rather than per-exception inside run()) guarantees a JSON failure document for every + # error reachable from the build, including bare-Exception template errors raised during + # __enter__/set_up (e.g. InvalidLayerReference, RemoteStackLocationNotSupported) that are + # not UserException subclasses. We re-raise unconditionally so @track_command still records + # telemetry and wraps non-UserException errors as UnhandledException, and so run()'s text + # "Build Failed" banner path is unaffected (no double-emit: run() no longer emits JSON). if OutputOption(output) is OutputOption.json: click.echo(build_failure_json(ex)) raise diff --git a/tests/unit/commands/buildcmd/test_build_context.py b/tests/unit/commands/buildcmd/test_build_context.py index ee8cb19f686..591c44aa803 100644 --- a/tests/unit/commands/buildcmd/test_build_context.py +++ b/tests/unit/commands/buildcmd/test_build_context.py @@ -1609,20 +1609,14 @@ def test_text_mode_prints_banner(self, echo_mock, secho_mock): echo_mock.assert_not_called() secho_mock.assert_called_once_with("\nBuild Failed", fg="red") - @parameterized.expand( - [ - (OutputOption.text, False), # text mode, banner explicitly suppressed - (OutputOption.json, True), # JSON mode: do_cli serializes, this method stays silent - ] - ) @patch("samcli.commands.build.build_context.click.secho") @patch("samcli.commands.build.build_context.click.echo") - def test_print_build_failure_stays_silent(self, output, print_banner, echo_mock, secho_mock): - self.build_context._output = output + def test_json_mode_stays_silent(self, echo_mock, secho_mock): + # In JSON mode _print_build_failure emits nothing (do_cli serializes the failure). + self.build_context._output = OutputOption.json - self.build_context._print_build_failure(print_text_banner=print_banner) + self.build_context._print_build_failure() - # Never emits JSON (that is do_cli's job); prints no banner in these two cases echo_mock.assert_not_called() secho_mock.assert_not_called() diff --git a/tests/unit/commands/buildcmd/test_command.py b/tests/unit/commands/buildcmd/test_command.py index 40f70490043..42e4c70b5f6 100644 --- a/tests/unit/commands/buildcmd/test_command.py +++ b/tests/unit/commands/buildcmd/test_command.py @@ -135,6 +135,14 @@ def test_json_failure_for_non_build_error_user_exception(self): self.assertEqual(result["status"], "failure") self.assertEqual(result["error"]["type"], "MissingBuildMethodException") + def test_json_failure_for_bare_exception(self): + # Bare-Exception template errors (e.g. InvalidLayerReference) are not UserException; + # the broad handler must still emit JSON so agent consumers never get empty stdout. + result = json.loads(self._run_expecting(RuntimeError("unexpected template error"))[0]) + + self.assertEqual(result["status"], "failure") + self.assertEqual(result["error"]["type"], "RuntimeError") + class TestGetModeValueFromEnvvar(TestCase): def setUp(self): From 050e1ec7eb3949eb12f27bb7f655bc255155c308 Mon Sep 17 00:00:00 2001 From: Madhav Donthula Date: Wed, 5 Aug 2026 17:52:00 -0700 Subject: [PATCH 17/22] fix: move option preprocessing inside do_cli try for JSON serialization process_env_var and process_image_options ran outside the try, so an InvalidImageException from a malformed --build-image (e.g. --build-image MyFunc=) exited 1 with empty stdout in --output json. Moving both inside the try routes them through the central JSON failure handler. --- samcli/commands/build/command.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/samcli/commands/build/command.py b/samcli/commands/build/command.py index d0a33613686..eb16b5ff9c3 100644 --- a/samcli/commands/build/command.py +++ b/samcli/commands/build/command.py @@ -249,10 +249,12 @@ def do_cli( # pylint: disable=too-many-locals, too-many-statements if use_container: LOG.info("Starting Build inside a container") - processed_env_vars = process_env_var(container_env_var) - processed_build_images = process_image_options(build_image) - try: + # Inside the try so option-preprocessing errors (e.g. InvalidImageException from a + # malformed --build-image) are serialized too, not just failures from within BuildContext. + processed_env_vars = process_env_var(container_env_var) + processed_build_images = process_image_options(build_image) + with BuildContext( function_identifier, template, From 20b09c0083724b41cd0572f843be8ade3b67d95f Mon Sep 17 00:00:00 2001 From: Madhav Donthula Date: Thu, 6 Aug 2026 10:59:29 -0700 Subject: [PATCH 18/22] docs: scope the JSON failure guarantee to execution errors The three comments claimed do_cli's handler serializes "every error reachable from the build". That is only true for execution failures once do_cli runs. Errors raised during click option processing (invalid flags, or a --hook-name prepare-hook failure re-raised by track_command) occur before do_cli and surface as click's standard usage/stderr output, not JSON. Soften the three comments to state the guarantee covers execution failures and note that invocation errors surface via click's stderr, so consumers treat "exit != 0 with empty stdout" as an invocation error. No behavior change. --- samcli/commands/build/build_context.py | 12 +++++++----- samcli/commands/build/command.py | 17 +++++++++++------ 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/samcli/commands/build/build_context.py b/samcli/commands/build/build_context.py index dbeff3ea951..0ffb79d3a06 100644 --- a/samcli/commands/build/build_context.py +++ b/samcli/commands/build/build_context.py @@ -74,8 +74,10 @@ def build_failure_json(ex: Exception) -> str: """Serialize a build failure into the structured JSON error document. - Single source of truth for the failure wire format, shared by do_cli (which handles - every failure path) and any other caller, so the schema lives in exactly one place. + Single source of truth for the failure wire format, shared by do_cli and any other + caller so the schema lives in exactly one place. This covers execution failures once + the command body runs; errors raised during click option processing (bad flags, hook + prepare failures) surface as click's standard usage/stderr output, not JSON. """ error_type = getattr(ex, "wrapped_from", None) or type(ex).__name__ return json.dumps( @@ -1161,9 +1163,9 @@ def _print_build_failure(self) -> None: """ Prints the human-readable "Build Failed" banner in text mode. - JSON-mode failure serialization is handled centrally in do_cli so that a single - handler covers every failure path (including exceptions raised before run()'s - try block, e.g. template parse errors or missing layer BuildMethod). + JSON-mode failure serialization is handled centrally in do_cli, which catches + execution failures raised from run()/set_up() (including exceptions raised before + run()'s try block, e.g. template parse errors or missing layer BuildMethod). """ if self._output is not OutputOption.json: click.secho("\nBuild Failed", fg="red") diff --git a/samcli/commands/build/command.py b/samcli/commands/build/command.py index eb16b5ff9c3..bfa0b327aa4 100644 --- a/samcli/commands/build/command.py +++ b/samcli/commands/build/command.py @@ -286,12 +286,17 @@ def do_cli( # pylint: disable=too-many-locals, too-many-statements ctx.run() except Exception as ex: # Central JSON failure serialization for sam build --output json. Catching broadly here - # (rather than per-exception inside run()) guarantees a JSON failure document for every - # error reachable from the build, including bare-Exception template errors raised during - # __enter__/set_up (e.g. InvalidLayerReference, RemoteStackLocationNotSupported) that are - # not UserException subclasses. We re-raise unconditionally so @track_command still records - # telemetry and wraps non-UserException errors as UnhandledException, and so run()'s text - # "Build Failed" banner path is unaffected (no double-emit: run() no longer emits JSON). + # (rather than per-exception inside run()) emits a JSON failure document for every + # execution error surfaced from BuildContext, including bare-Exception template errors + # raised during __enter__/set_up (e.g. InvalidLayerReference, RemoteStackLocationNotSupported) + # that are not UserException subclasses. We re-raise unconditionally so @track_command still + # records telemetry and wraps non-UserException errors as UnhandledException, and so run()'s + # text "Build Failed" banner path is unaffected (no double-emit: run() no longer emits JSON). + # + # Scope: this covers failures once do_cli runs. Errors raised earlier during click option + # processing (invalid flags, or a --hook-name prepare-hook failure re-raised by + # track_command) surface as click's standard usage/stderr output, not JSON. Consumers should + # treat "exit != 0 with empty stdout" as an invocation error, not an execution failure. if OutputOption(output) is OutputOption.json: click.echo(build_failure_json(ex)) raise From a64f926873f9d3a3b8e002abdebf1a4881b90602 Mon Sep 17 00:00:00 2001 From: Madhav Donthula Date: Thu, 6 Aug 2026 11:41:26 -0700 Subject: [PATCH 19/22] feat: add package_type discriminator; don't blame resource-agnostic failures - Add package_type to each function in the JSON success document so Image functions (runtime: null) are distinguishable from Zip, via a new _function_to_json helper. - Restrict the resource_name fallback: InvalidBuildGraphException and UnsupportedBuilderLibraryVersionError are not resource-specific, so they keep resource=None instead of being attributed to whatever resource the user named. - Document that error.resource is a representative id when functions share a build definition (points to get_resource_full_paths for the full set). - Tests: package_type for image functions; per-type resource_name expectations. --- samcli/commands/build/build_context.py | 38 +++++++++++++------ samcli/lib/build/build_strategy.py | 4 ++ .../commands/buildcmd/test_build_context.py | 32 ++++++++++++---- 3 files changed, 56 insertions(+), 18 deletions(-) diff --git a/samcli/commands/build/build_context.py b/samcli/commands/build/build_context.py index 0ffb79d3a06..bb477b0b8fc 100644 --- a/samcli/commands/build/build_context.py +++ b/samcli/commands/build/build_context.py @@ -55,7 +55,7 @@ _resolve_property_paths, _set_prop_value, ) -from samcli.lib.providers.provider import LayerVersion, ResourcesToBuildCollector, Stack, get_full_path +from samcli.lib.providers.provider import Function, LayerVersion, ResourcesToBuildCollector, Stack, get_full_path from samcli.lib.providers.sam_api_provider import SamApiProvider from samcli.lib.providers.sam_function_provider import SamFunctionProvider from samcli.lib.providers.sam_layer_provider import SamLayerProvider @@ -393,9 +393,16 @@ def run(self) -> None: self._print_build_failure() user_ex = UserException(str(ex), wrapped_from=wrapped_from) - # Prefer the resource the exception attributes the failure to; otherwise fall back to - # the resource the user asked to build (relevant even when it doesn't exist). - user_ex.resource_name = getattr(ex, "resource_name", None) or self._resource_identifier + # Prefer the resource the exception attributes the failure to. Otherwise fall back to + # the resource the user asked to build - but only for failures actually tied to a + # resource. Resource-agnostic errors (corrupt build graph, outdated builder container) + # keep resource=None rather than blaming whatever resource the user happened to name. + resource_name = getattr(ex, "resource_name", None) + if resource_name is None and not isinstance( + ex, (InvalidBuildGraphException, UnsupportedBuilderLibraryVersionError) + ): + resource_name = self._resource_identifier + user_ex.resource_name = resource_name raise user_ex from ex finally: if self.build_in_source: @@ -1106,6 +1113,21 @@ def _copy_artifact_paths(self, original_resource: Dict, modified_resource: Dict) if value is not None: _set_prop_value(original_props, prop_name, value) + @staticmethod + def _function_to_json(function: Function) -> Dict[str, Any]: + """ + Serialize a built function for the JSON success document. Includes a package_type + discriminator so Image functions (which have runtime: None) are distinguishable from + Zip functions rather than both appearing as runtime: null. + """ + return { + "resource_id": function.full_path, + "type": "function", + "package_type": function.packagetype, + "runtime": function.runtime, + "architecture": function.architecture, + } + def _print_build_success( self, artifacts_dir: str, output_template_path: str, resources_to_build: ResourcesToBuildCollector ) -> None: @@ -1123,13 +1145,7 @@ def _print_build_success( """ if self._output is OutputOption.json: resources: List[Dict[str, Any]] = [ - { - "resource_id": function.full_path, - "type": "function", - "runtime": function.runtime, - "architecture": function.architecture, - } - for function in resources_to_build.functions + self._function_to_json(function) for function in resources_to_build.functions ] + [ { "resource_id": layer.full_path, diff --git a/samcli/lib/build/build_strategy.py b/samcli/lib/build/build_strategy.py index bda16c8faa6..67ad61dae49 100644 --- a/samcli/lib/build/build_strategy.py +++ b/samcli/lib/build/build_strategy.py @@ -160,6 +160,10 @@ def build_single_function_definition(self, build_definition: FunctionBuildDefini UnsupportedBuilderLibraryVersionError, ) as ex: if getattr(ex, "resource_name", None) is None: + # Representative resource id. Functions sharing runtime + CodeUri + metadata + # collapse into one build definition, so a single failure can affect several; + # get_full_path() names the first. get_resource_full_paths() lists all if a + # consumer needs the full set. ex.resource_name = build_definition.get_full_path() raise diff --git a/tests/unit/commands/buildcmd/test_build_context.py b/tests/unit/commands/buildcmd/test_build_context.py index 591c44aa803..f0ecd8ad836 100644 --- a/tests/unit/commands/buildcmd/test_build_context.py +++ b/tests/unit/commands/buildcmd/test_build_context.py @@ -1154,13 +1154,17 @@ def test_run_json_mode_emits_single_parseable_document( @parameterized.expand( [ - (UnsupportedRuntimeException(), "UnsupportedRuntimeException"), - (UnsupportedBuilderException(), "UnsupportedBuilderException"), - (BuildInsideContainerError(), "BuildInsideContainerError"), - (BuildError(wrapped_from=DeepWrap().__class__.__name__, msg="Test"), "DeepWrap"), + # (exception, expected wrapped_from, expected resource_name) + # Resource-tied failures fall back to the requested resource id ("function_identifier"); + # resource-agnostic ones (outdated builder container) keep resource=None. + (UnsupportedRuntimeException(), "UnsupportedRuntimeException", "function_identifier"), + (UnsupportedBuilderException(), "UnsupportedBuilderException", "function_identifier"), + (BuildInsideContainerError(), "BuildInsideContainerError", "function_identifier"), + (BuildError(wrapped_from=DeepWrap().__class__.__name__, msg="Test"), "DeepWrap", "function_identifier"), ( UnsupportedBuilderLibraryVersionError(container_name="name", error_msg="msg"), "UnsupportedBuilderLibraryVersionError", + None, ), ] ) @@ -1181,6 +1185,7 @@ def test_must_catch_known_exceptions( self, exception, wrapped_exception, + expected_resource_name, esbuild_bundler_manager_mock, os_mock, get_template_data_mock, @@ -1244,9 +1249,9 @@ def test_must_catch_known_exceptions( self.assertEqual(str(ctx.exception), str(exception)) self.assertEqual(wrapped_exception, ctx.exception.wrapped_from) - # None of these exceptions carry resource_name, so the re-raised UserException falls - # back to the resource the user asked to build. - self.assertEqual(ctx.exception.resource_name, "function_identifier") + # Resource-tied failures fall back to the requested resource id; resource-agnostic ones + # (e.g. outdated builder container) keep resource=None so no innocent resource is blamed. + self.assertEqual(ctx.exception.resource_name, expected_resource_name) @patch("samcli.commands.build.build_context.SamLocalStackProvider.get_stacks") @patch("samcli.commands.build.build_context.SamApiProvider") @@ -1538,6 +1543,19 @@ def test_json_empty_collector(self, echo_mock): result = json.loads(echo_mock.call_args[0][0]) self.assertEqual(result["resources"], []) + @patch("samcli.commands.build.build_context.click.echo") + def test_json_image_function_has_package_type_discriminator(self, echo_mock): + # Image functions have runtime: None; package_type lets a consumer distinguish them + # from a malformed Zip entry. + image_fn = get_function("ImgFn", runtime=None, packagetype=IMAGE) + collector = self._collector(functions=[image_fn]) + + self.build_context._print_build_success("artifacts", "out_template", collector) + + resource = json.loads(echo_mock.call_args[0][0])["resources"][0] + self.assertEqual(resource["package_type"], "Image") + self.assertIsNone(resource["runtime"]) + @patch("samcli.commands.build.build_context.click.echo") def test_json_architecture_defaults_to_x86_64_when_absent(self, echo_mock): # get_function() builds a Function with architectures=None From 8c485b03f55e95f2ffd90b79cc482e862a9d0666 Mon Sep 17 00:00:00 2001 From: Madhav Donthula Date: Thu, 6 Aug 2026 12:07:17 -0700 Subject: [PATCH 20/22] Resolve error.resource to full path for namespace consistency The JSON success document reports resources[].resource_id as the resource's full_path (nested-stack-qualified, e.g. ChildStack/MyFn), but the failure fallback reported the raw CLI identifier (MyFn). Consumers could not correlate a failure back to the resource_id they saw on success. Add _resolve_resource_full_path(), which looks the identifier up via the same function/layer providers the success document uses and returns its full_path, falling back to the raw identifier only when lookup fails. Both fallback paths (FunctionNotFound and the general handler) now route through it. --- samcli/commands/build/build_context.py | 27 ++++++++++++++++--- .../commands/buildcmd/test_build_context.py | 23 +++++++++++----- 2 files changed, 41 insertions(+), 9 deletions(-) diff --git a/samcli/commands/build/build_context.py b/samcli/commands/build/build_context.py index bb477b0b8fc..a77e36be711 100644 --- a/samcli/commands/build/build_context.py +++ b/samcli/commands/build/build_context.py @@ -370,8 +370,11 @@ def run(self) -> None: caught_exception = function_not_found_ex user_ex = UserException(str(function_not_found_ex), wrapped_from=function_not_found_ex.__class__.__name__) - # The failure is attributable to the specific resource the user asked to build. - user_ex.resource_name = getattr(function_not_found_ex, "resource_name", None) or self._resource_identifier + # The failure is attributable to the specific resource the user asked to build. Resolve + # it to a full path so it matches the resource_id namespace of the success document. + user_ex.resource_name = getattr( + function_not_found_ex, "resource_name", None + ) or self._resolve_resource_full_path(self._resource_identifier) raise user_ex from function_not_found_ex except ( UnsupportedRuntimeException, @@ -401,7 +404,8 @@ def run(self) -> None: if resource_name is None and not isinstance( ex, (InvalidBuildGraphException, UnsupportedBuilderLibraryVersionError) ): - resource_name = self._resource_identifier + # Resolve to a full path so it matches the resource_id namespace of the success document. + resource_name = self._resolve_resource_full_path(self._resource_identifier) user_ex.resource_name = resource_name raise user_ex from ex finally: @@ -1113,6 +1117,23 @@ def _copy_artifact_paths(self, original_resource: Dict, modified_resource: Dict) if value is not None: _set_prop_value(original_props, prop_name, value) + def _resolve_resource_full_path(self, resource_identifier: Optional[str]) -> Optional[str]: + """ + Resolve a resource identifier (a CLI argument, which may be a bare logical ID) to its + full path, so error.resource stays in the same namespace as resources[].resource_id in + the success document (both nested-stack-qualified, e.g. ChildStack/MyFn). Falls back to + the raw identifier if the resource cannot be looked up. + """ + if not resource_identifier: + return resource_identifier + function = self.function_provider.get(resource_identifier) if self.function_provider else None + if function: + return function.full_path + layer = self.layer_provider.get(resource_identifier) if self.layer_provider else None + if layer: + return layer.full_path + return resource_identifier + @staticmethod def _function_to_json(function: Function) -> Dict[str, Any]: """ diff --git a/tests/unit/commands/buildcmd/test_build_context.py b/tests/unit/commands/buildcmd/test_build_context.py index f0ecd8ad836..06311ba4a5e 100644 --- a/tests/unit/commands/buildcmd/test_build_context.py +++ b/tests/unit/commands/buildcmd/test_build_context.py @@ -1155,12 +1155,12 @@ def test_run_json_mode_emits_single_parseable_document( @parameterized.expand( [ # (exception, expected wrapped_from, expected resource_name) - # Resource-tied failures fall back to the requested resource id ("function_identifier"); - # resource-agnostic ones (outdated builder container) keep resource=None. - (UnsupportedRuntimeException(), "UnsupportedRuntimeException", "function_identifier"), - (UnsupportedBuilderException(), "UnsupportedBuilderException", "function_identifier"), - (BuildInsideContainerError(), "BuildInsideContainerError", "function_identifier"), - (BuildError(wrapped_from=DeepWrap().__class__.__name__, msg="Test"), "DeepWrap", "function_identifier"), + # Resource-tied failures resolve the requested identifier to its full path (the mocked + # function provider returns func1); resource-agnostic ones keep resource=None. + (UnsupportedRuntimeException(), "UnsupportedRuntimeException", "func1"), + (UnsupportedBuilderException(), "UnsupportedBuilderException", "func1"), + (BuildInsideContainerError(), "BuildInsideContainerError", "func1"), + (BuildError(wrapped_from=DeepWrap().__class__.__name__, msg="Test"), "DeepWrap", "func1"), ( UnsupportedBuilderLibraryVersionError(container_name="name", error_msg="msg"), "UnsupportedBuilderLibraryVersionError", @@ -1638,6 +1638,17 @@ def test_json_mode_stays_silent(self, echo_mock, secho_mock): echo_mock.assert_not_called() secho_mock.assert_not_called() + def test_resolves_bare_id_to_full_path(self): + # A nested-stack function's full_path differs from the bare CLI id; resolving keeps + # error.resource in the same namespace as the success document's resource_id. + nested = get_function("MyFn")._replace(stack_path="ChildStack") + self.build_context._function_provider = Mock() + self.build_context._function_provider.get.return_value = nested + self.build_context._layer_provider = Mock() + self.build_context._layer_provider.get.return_value = None + + self.assertEqual(self.build_context._resolve_resource_full_path("MyFn"), "ChildStack/MyFn") + class TestBuildContext_check_build_method_experimental_flag(TestCase): def setUp(self): From 5ad0867d226047bcaf22a66720a749e8aca86d82 Mon Sep 17 00:00:00 2001 From: Madhav Donthula Date: Thu, 6 Aug 2026 13:13:27 -0700 Subject: [PATCH 21/22] Correct representative-resource comment for JSON contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The comment pointed JSON consumers at get_resource_full_paths() as an escape hatch for the full set of collapsed functions, but that method is internal and nothing in the failure document exposes it. Naming one representative is the deliberate contract — error.resource is a single value. Update the comment to say so and stop describing an escape hatch that does not exist for this output. --- samcli/lib/build/build_strategy.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/samcli/lib/build/build_strategy.py b/samcli/lib/build/build_strategy.py index 67ad61dae49..67fd9cd20da 100644 --- a/samcli/lib/build/build_strategy.py +++ b/samcli/lib/build/build_strategy.py @@ -162,8 +162,8 @@ def build_single_function_definition(self, build_definition: FunctionBuildDefini if getattr(ex, "resource_name", None) is None: # Representative resource id. Functions sharing runtime + CodeUri + metadata # collapse into one build definition, so a single failure can affect several; - # get_full_path() names the first. get_resource_full_paths() lists all if a - # consumer needs the full set. + # get_full_path() names the first. Reporting one representative is the deliberate + # contract here — the JSON failure document carries a single error.resource. ex.resource_name = build_definition.get_full_path() raise From 77f417ff1a39a72eb822839759af99a082888bd1 Mon Sep 17 00:00:00 2001 From: Madhav Donthula Date: Thu, 6 Aug 2026 13:46:54 -0700 Subject: [PATCH 22/22] Skip interactive prompts in JSON output mode sam build --use-container --output json could hit two blocking confirms that a non-interactive consumer cannot answer: the mount-with-write prompt and the python-uv beta-feature confirm. On dead stdin click.confirm raises EOFError -> click.Abort, which the broad do_cli handler serialized as {"type": "Abort", "message": "", "resource": null} - a valid-looking failure document with no diagnosable content. Skip both prompts in JSON mode: the mount prompt falls back to the documented READ-only default, and the beta confirm is skipped unless the feature is already enabled (where prompt_experimental only updates telemetry context). Skipping the mount pre-check also fixes attribution for unsupported-runtime errors: get_workflow_config in build/utils.py ran outside the resource-tagging wrappers and reported resource=null on a full build. With the pre-check skipped, the same exception surfaces from ApplicationBuilder._build_function, which is inside build_single_function_definition's wrapper, so error.resource is populated - no change to build/utils.py needed. --- samcli/commands/build/build_context.py | 16 ++++-- .../commands/buildcmd/test_build_context.py | 51 +++++++++++++++++++ 2 files changed, 63 insertions(+), 4 deletions(-) diff --git a/samcli/commands/build/build_context.py b/samcli/commands/build/build_context.py index a77e36be711..86e1f562125 100644 --- a/samcli/commands/build/build_context.py +++ b/samcli/commands/build/build_context.py @@ -15,7 +15,7 @@ import click from samcli.commands._utils.constants import DEFAULT_BUILD_DIR -from samcli.commands._utils.experimental import ExperimentalFlag, prompt_experimental +from samcli.commands._utils.experimental import ExperimentalFlag, is_experimental_enabled, prompt_experimental from samcli.commands._utils.template import ( FOREACH_REQUIRED_ELEMENTS, get_template_data, @@ -309,9 +309,11 @@ def run(self) -> None: if self._use_container: if self._mount_with == MountMode.WRITE: mount_with_write = True - else: + elif self._output is not OutputOption.json: # if self._mount_with is NOT WRITE - # check the need of mounting with write permissions and prompt user to enable it if needed + # check the need of mounting with write permissions and prompt user to enable it if needed. + # Skipped in JSON mode: a non-interactive consumer cannot answer the confirm, so fall back + # to the documented READ-only default rather than blocking on stdin (which aborts the build). mount_with_write = prompt_user_to_enable_mount_with_write_if_needed( resources_to_build, self.base_dir, @@ -1520,13 +1522,19 @@ def _check_build_method_experimental_flag(self) -> None: for function in resources_to_build.functions: if function.metadata and function.metadata.get("BuildMethod", "") in EXPERIMENTAL_BUILD_METHODS: build_method = function.metadata.get("BuildMethod", "") + experimental_flag = EXPERIMENTAL_BUILD_METHODS[build_method] + # A JSON consumer cannot answer the interactive beta confirmation. Skip it unless the + # feature is already enabled (via --beta-features / env), in which case prompt_experimental + # just updates the telemetry context and returns without prompting. + if self._output is OutputOption.json and not is_experimental_enabled(experimental_flag): + continue WARNING_MESSAGE = ( f'Build method "{build_method}" is a beta feature.\n' "Please confirm if you would like to proceed\n" 'You can also enable this beta feature with "sam build --beta-features".' ) - prompt_experimental(EXPERIMENTAL_BUILD_METHODS[build_method], WARNING_MESSAGE) + prompt_experimental(experimental_flag, WARNING_MESSAGE) @property def build_in_source(self) -> Optional[bool]: diff --git a/tests/unit/commands/buildcmd/test_build_context.py b/tests/unit/commands/buildcmd/test_build_context.py index 06311ba4a5e..e29e4b1d9ff 100644 --- a/tests/unit/commands/buildcmd/test_build_context.py +++ b/tests/unit/commands/buildcmd/test_build_context.py @@ -1374,6 +1374,39 @@ def test_build_in_source_event_sent( EventName.USED_FEATURE.value, UsedFeature.BUILD_IN_SOURCE.value, "FunctionNotFound" ) + @patch("samcli.commands.build.build_context.prompt_user_to_enable_mount_with_write_if_needed") + @patch("samcli.commands.build.build_context.BuildContext._is_sam_template", return_value=False) + @patch("samcli.commands.build.build_context.BuildContext._handle_build_pre_processing") + @patch("samcli.commands.build.build_context.BuildContext.get_resources_to_build") + @patch("samcli.commands.build.build_context.BuildContext._check_exclude_warning") + @patch("samcli.commands.build.build_context.BuildContext._check_build_method_experimental_flag") + @patch("samcli.lib.build.app_builder.ApplicationBuilder.build") + def test_skips_mount_prompt_in_json_mode( + self, mock_build, mock_experimental, mock_warning, mock_get_resources, mock_pre_processing, _, mock_prompt + ): + # --use-container without --mount-with WRITE normally prompts on stdin. In JSON mode a + # non-interactive consumer cannot answer, so the prompt is skipped (defaulting to READ-only) + # rather than aborting the build. build() raises to short-circuit right after the decision. + mock_build.side_effect = FunctionNotFound() + context = BuildContext( + resource_identifier="", + template_file="template_file", + base_dir="base_dir", + build_dir="build_dir", + cache_dir="cache_dir", + cached=False, + parallel=False, + mode="mode", + use_container=True, + mount_with=MountMode.READ.value, + output="json", + ) + + with self.assertRaises(UserException): + context.run() + + mock_prompt.assert_not_called() + class TestBuildContext_is_sam_template(TestCase): @parameterized.expand( @@ -1702,6 +1735,24 @@ def test_check_build_method_experimental_flag_no_metadata(self, mock_get_resourc mock_prompt.assert_not_called() + @patch("samcli.commands.build.build_context.is_experimental_enabled", return_value=False) + @patch("samcli.commands.build.build_context.prompt_experimental") + @patch("samcli.commands.build.build_context.BuildContext.get_resources_to_build") + def test_skips_beta_prompt_in_json_mode_when_not_enabled(self, mock_get_resources, mock_prompt, _): + # A JSON consumer cannot answer the confirm; skip it (rather than aborting on EOF) and let the + # non-gating beta build proceed. If the flag were already enabled, prompt_experimental would run + # to update telemetry context - covered by is_experimental_enabled=False here. + self.build_context._output = OutputOption.json + mock_function = Mock() + mock_function.metadata = {"BuildMethod": "python-uv"} + mock_resources = Mock() + mock_resources.functions = [mock_function] + mock_get_resources.return_value = mock_resources + + self.build_context._check_build_method_experimental_flag() + + mock_prompt.assert_not_called() + class TestBuildContext_get_template_for_output(TestCase): """Tests for the _get_template_for_output method that handles original template preservation."""