Skip to content

Commit 6956c10

Browse files
committed
fix: narrow exception handling in child template LE expansion
The previous code caught bare Exception when expanding language extensions on a nested-stack child template, downgrading every failure (including SAM CLI bugs) to a single WARNING line. That masked real defects: any crash in the new library silently fell through to the non-expanded path, leaving the packaged child with unresolved local paths that CloudFormation could not resolve at deploy time. Two changes: 1. InvalidSamDocumentException (the documented failure surface of expand_language_extensions) keeps the warn-and-fallback behavior, but the message now explains the consequence: artifact URIs inside Fn::ForEach blocks will NOT be uploaded. 2. Any other exception is logged at ERROR with traceback (exc_info=True) and a pointer to file an issue. We still fall back so the rest of sam package keeps going, but the failure is observable to users running --debug and to telemetry instead of silently degrading. Also removed the check_using_language_extension branch that was dead: expand_language_extensions only raises InvalidSamDocumentException when it actually tried to expand, which implies the transform was present. Tests: - TestCloudFormationStackResourceExpansionErrorHandling covers both branches (warn vs error) and confirms the non-expanded fallback is still taken.
1 parent b5b5253 commit 6956c10

2 files changed

Lines changed: 107 additions & 7 deletions

File tree

samcli/lib/package/artifact_exporter.py

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,6 @@ def do_export(self, resource_id, resource_dict, parent_dir):
122122
(preserving the Fn::ForEach structure) before uploading.
123123
"""
124124
from samcli.lib.cfn_language_extensions.sam_integration import (
125-
check_using_language_extension,
126125
expand_language_extensions,
127126
)
128127
from samcli.lib.intrinsic_resolver.intrinsics_symbol_table import IntrinsicsSymbolTable
@@ -170,13 +169,34 @@ def do_export(self, resource_id, resource_dict, parent_dir):
170169
try:
171170
result = expand_language_extensions(child_template_dict, parameter_values, template_path=abs_template_path)
172171
except InvalidSamDocumentException as e:
173-
if check_using_language_extension(child_template_dict):
174-
LOG.warning("Language extensions expansion failed for %s: %s", abs_template_path, e)
175-
else:
176-
LOG.debug("Language extensions expansion failed for %s, using original template", abs_template_path)
172+
# Expected failure path: the child template triggered the
173+
# AWS::LanguageExtensions transform but SAM CLI could not expand it
174+
# locally. Defer to CloudFormation's server-side transform. Note: any
175+
# artifact paths inside Fn::ForEach bodies will NOT be uploaded, so
176+
# child stacks with dynamic artifact properties (e.g. CodeUri using a
177+
# loop variable) will fail at deploy time.
178+
LOG.warning(
179+
"Language extensions expansion failed for %s. "
180+
"CloudFormation will process the AWS::LanguageExtensions transform "
181+
"server-side; artifact URIs inside Fn::ForEach blocks will NOT be "
182+
"uploaded. Error: %s",
183+
abs_template_path,
184+
e,
185+
)
177186
result = None
178-
except Exception as e:
179-
LOG.warning("Unexpected error during language extensions expansion for %s: %s", abs_template_path, e)
187+
except Exception as e: # pylint: disable=broad-except
188+
# Unexpected failure: a bug in SAM CLI rather than a malformed template.
189+
# Surface at ERROR with a traceback so users running --debug (and SAM
190+
# telemetry) see it, but don't abort the rest of the package run.
191+
LOG.error(
192+
"Internal error expanding language extensions for %s. This is a "
193+
"SAM CLI bug; please report at "
194+
"https://github.com/aws/aws-sam-cli/issues with the template. "
195+
"Falling back to server-side transform. Error: %s",
196+
abs_template_path,
197+
e,
198+
exc_info=True,
199+
)
180200
result = None
181201

182202
if result and result.had_language_extensions:

tests/unit/lib/package/test_artifact_exporter.py

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2694,3 +2694,83 @@ def test_child_template_receives_parent_parameters(self):
26942694
import shutil
26952695

26962696
shutil.rmtree(tmpdir, ignore_errors=True)
2697+
2698+
2699+
class TestCloudFormationStackResourceExpansionErrorHandling(unittest.TestCase):
2700+
"""Error-path behavior for language-extensions expansion in do_export.
2701+
2702+
Issue #2 in the review: the broad `except Exception` was downgrading
2703+
SAM CLI bugs to warnings, silently masking defects. The new behavior:
2704+
- InvalidSamDocumentException: warn (expected user-facing failure)
2705+
- Other exceptions: log ERROR with traceback (bug, not user input)
2706+
Both still fall back to the non-expanded path so the rest of sam
2707+
package keeps going.
2708+
"""
2709+
2710+
def _make_stack_resource(self):
2711+
uploaders_mock = Mock()
2712+
uploaders_mock.get.return_value = Mock()
2713+
uploaders_mock.get.return_value.upload.return_value = "s3://bucket/key"
2714+
uploaders_mock.get.return_value.to_path_style_s3_url.return_value = "http://s3.amazonaws.com/bucket/key"
2715+
code_signer_mock = Mock()
2716+
code_signer_mock.should_sign_package.return_value = False
2717+
return CloudFormationStackResource(uploaders_mock, code_signer_mock)
2718+
2719+
def _write_child(self, tmpdir):
2720+
child = {
2721+
"AWSTemplateFormatVersion": "2010-09-09",
2722+
"Transform": "AWS::LanguageExtensions",
2723+
"Resources": {"Dummy": {"Type": "AWS::SNS::Topic", "Properties": {}}},
2724+
}
2725+
from samcli.yamlhelper import yaml_dump
2726+
2727+
path = os.path.join(tmpdir, "child.yaml")
2728+
with open(path, "w", encoding="utf-8") as f:
2729+
f.write(yaml_dump(child))
2730+
return path
2731+
2732+
def test_invalid_sam_document_exception_logs_warning_and_falls_back(self):
2733+
stack_resource = self._make_stack_resource()
2734+
tmpdir = tempfile.mkdtemp()
2735+
try:
2736+
child_path = self._write_child(tmpdir)
2737+
with (
2738+
patch("samcli.lib.cfn_language_extensions.sam_integration.expand_language_extensions") as expand_mock,
2739+
patch("samcli.lib.package.artifact_exporter.Template") as TemplateMock,
2740+
self.assertLogs("samcli.lib.package.artifact_exporter", level="WARNING") as log_ctx,
2741+
):
2742+
expand_mock.side_effect = InvalidSamDocumentException("bad template")
2743+
TemplateMock.return_value.export.return_value = {"Resources": {}}
2744+
stack_resource.do_export("ChildStack", {"TemplateURL": child_path}, tmpdir)
2745+
2746+
# Warning emitted (not error) and fallback path taken.
2747+
self.assertTrue(any("Language extensions expansion failed" in m for m in log_ctx.output))
2748+
TemplateMock.assert_called_once() # non-extension fallback constructor
2749+
finally:
2750+
import shutil
2751+
2752+
shutil.rmtree(tmpdir, ignore_errors=True)
2753+
2754+
def test_unexpected_exception_logs_error_and_falls_back(self):
2755+
stack_resource = self._make_stack_resource()
2756+
tmpdir = tempfile.mkdtemp()
2757+
try:
2758+
child_path = self._write_child(tmpdir)
2759+
with (
2760+
patch("samcli.lib.cfn_language_extensions.sam_integration.expand_language_extensions") as expand_mock,
2761+
patch("samcli.lib.package.artifact_exporter.Template") as TemplateMock,
2762+
self.assertLogs("samcli.lib.package.artifact_exporter", level="ERROR") as log_ctx,
2763+
):
2764+
expand_mock.side_effect = AttributeError("unexpected bug")
2765+
TemplateMock.return_value.export.return_value = {"Resources": {}}
2766+
stack_resource.do_export("ChildStack", {"TemplateURL": child_path}, tmpdir)
2767+
2768+
# ERROR-level log containing the bug-report pointer; fallback still taken.
2769+
joined = "\n".join(log_ctx.output)
2770+
self.assertIn("Internal error expanding language extensions", joined)
2771+
self.assertIn("github.com/aws/aws-sam-cli/issues", joined)
2772+
TemplateMock.assert_called_once()
2773+
finally:
2774+
import shutil
2775+
2776+
shutil.rmtree(tmpdir, ignore_errors=True)

0 commit comments

Comments
 (0)