Skip to content

Commit

Permalink
feat: Update handling of UserExceptions that are raised (aws#1686)
Browse files Browse the repository at this point in the history
  • Loading branch information
jfuss authored and awood45 committed Dec 21, 2019
1 parent 23d2449 commit af124bc
Show file tree
Hide file tree
Showing 19 changed files with 101 additions and 46 deletions.
13 changes: 11 additions & 2 deletions samcli/commands/_utils/template.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,19 @@
import pathlib
import yaml

from samcli.commands.exceptions import UserException
from samcli.yamlhelper import yaml_parse, yaml_dump
from samcli.commands._utils.resources import METADATA_WITH_LOCAL_PATHS, RESOURCES_WITH_LOCAL_PATHS


class TemplateNotFoundException(UserException):
pass


class TemplateFailedParsingException(UserException):
pass


def get_template_data(template_file):
"""
Read the template file, parse it as JSON/YAML and return the template as a dictionary.
Expand All @@ -25,13 +34,13 @@ def get_template_data(template_file):
"""

if not pathlib.Path(template_file).exists():
raise ValueError("Template file not found at {}".format(template_file))
raise TemplateNotFoundException("Template file not found at {}".format(template_file))

with open(template_file, "r") as fp:
try:
return yaml_parse(fp.read())
except (ValueError, yaml.YAMLError) as ex:
raise ValueError("Failed to parse template: {}".format(str(ex)))
raise TemplateFailedParsingException("Failed to parse template: {}".format(str(ex)))


def move_template(src_template_path, dest_template_path, template_dict):
Expand Down
5 changes: 1 addition & 4 deletions samcli/commands/build/build_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,7 @@ def __init__(
self._container_manager = None

def __enter__(self):
try:
self._template_dict = get_template_data(self._template_file)
except ValueError as ex:
raise UserException(str(ex))
self._template_dict = get_template_data(self._template_file)

self._function_provider = SamFunctionProvider(self._template_dict, self._parameter_overrides)

Expand Down
4 changes: 2 additions & 2 deletions samcli/commands/build/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ def do_cli( # pylint: disable=too-many-locals, too-many-statements
mode=ctx.mode,
)
except FunctionNotFound as ex:
raise UserException(str(ex))
raise UserException(str(ex), wrapped_from=ex.__class__.__name__)

try:
artifacts = builder.build()
Expand Down Expand Up @@ -222,7 +222,7 @@ def do_cli( # pylint: disable=too-many-locals, too-many-statements
ContainerBuildNotSupported,
) as ex:
click.secho("\nBuild Failed", fg="red")
raise UserException(str(ex))
raise UserException(str(ex), wrapped_from=ex.__class__.__name__)


def gen_success_msg(artifacts_dir, output_template_path, is_default_build_dir):
Expand Down
5 changes: 5 additions & 0 deletions samcli/commands/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ class UserException(click.ClickException):

exit_code = 1

def __init__(self, message, wrapped_from=None):
self.wrapped_from = wrapped_from

click.ClickException.__init__(self, message)


class CredentialsError(UserException):
"""
Expand Down
6 changes: 3 additions & 3 deletions samcli/commands/init/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ def do_cli(
--name and --runtime and --app-template and --dependency-manager
--location
"""
raise UserException(msg)
raise click.UsageError(msg)
# check for required parameters
if location or (name and runtime and dependency_manager and app_template):
# need to turn app_template into a location before we generate
Expand All @@ -170,7 +170,7 @@ def do_cli(
You can also re-run without the --no-interactive flag to be prompted for required values.
"""
raise UserException(error_msg)
raise click.UsageError(error_msg)
else:
# proceed to interactive state machine, which will call do_generate
do_interactive(location, runtime, dependency_manager, output_dir, name, app_template, no_input)
Expand All @@ -190,7 +190,7 @@ def _get_cookiecutter_template_context(name, runtime, extra_context):
try:
extra_context_dict = json.loads(extra_context)
except JSONDecodeError:
raise UserException(
raise click.UsageError(
"Parse error reading the --extra-context parameter. The value of this parameter must be valid JSON."
)

Expand Down
2 changes: 1 addition & 1 deletion samcli/commands/init/init_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,4 @@ def do_generate(location, runtime, dependency_manager, output_dir, name, no_inpu
try:
generate_project(location, runtime, dependency_manager, output_dir, name, no_input, extra_context)
except (GenerateProjectFailedError, ArbitraryProjectDownloadFailed) as e:
raise UserException(str(e))
raise UserException(str(e), wrapped_from=e.__class__.__name__)
12 changes: 8 additions & 4 deletions samcli/commands/init/init_templates.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@
LOG = logging.getLogger(__name__)


class InvalidInitTemplateError(UserException):
pass


class InitTemplates:
def __init__(self, no_interactive=False, auto_clone=True):
self._repo_url = "https://github.com/awslabs/aws-sam-cli-app-templates.git"
Expand Down Expand Up @@ -61,7 +65,7 @@ def prompt_for_location(self, runtime, dependency_manager):
return (template_md["init_location"], "hello-world")
if template_md.get("directory") is not None:
return (os.path.join(self.repo_path, template_md["directory"]), template_md["appTemplate"])
raise UserException("Invalid template. This should not be possible, please raise an issue.")
raise InvalidInitTemplateError("Invalid template. This should not be possible, please raise an issue.")

def location_from_app_template(self, runtime, dependency_manager, app_template):
options = self.init_options(runtime, dependency_manager)
Expand All @@ -71,10 +75,10 @@ def location_from_app_template(self, runtime, dependency_manager, app_template):
return template["init_location"]
if template.get("directory") is not None:
return os.path.join(self.repo_path, template["directory"])
raise UserException("Invalid template. This should not be possible, please raise an issue.")
raise InvalidInitTemplateError("Invalid template. This should not be possible, please raise an issue.")
except StopIteration:
msg = "Can't find application template " + app_template + " - check valid values in interactive init."
raise UserException(msg)
raise InvalidInitTemplateError(msg)

def _check_app_template(self, entry, app_template):
return entry["appTemplate"] == app_template
Expand Down Expand Up @@ -109,7 +113,7 @@ def _init_options_from_bundle(self, runtime, dependency_manager):
msg = "Lambda Runtime {} and dependency manager {} does not have an available initialization template.".format(
runtime, dependency_manager
)
raise UserException(msg)
raise InvalidInitTemplateError(msg)

def _shared_dir_check(self, shared_dir):
try:
Expand Down
4 changes: 2 additions & 2 deletions samcli/commands/local/cli_common/invoke_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from samcli.local.lambdafn.runtime import LambdaRuntime
from samcli.local.docker.lambda_image import LambdaImage
from samcli.local.docker.manager import ContainerManager
from samcli.commands._utils.template import get_template_data
from samcli.commands._utils.template import get_template_data, TemplateNotFoundException, TemplateFailedParsingException
from samcli.local.layers.layer_downloader import LayerDownloader
from samcli.lib.providers.sam_function_provider import SamFunctionProvider
from .user_exceptions import InvokeContextException, DebugContextException
Expand Down Expand Up @@ -273,7 +273,7 @@ def _get_template_data(template_file):

try:
return get_template_data(template_file)
except ValueError as ex:
except (TemplateNotFoundException, TemplateFailedParsingException) as ex:
raise InvokeContextException(str(ex))

@staticmethod
Expand Down
10 changes: 6 additions & 4 deletions samcli/commands/local/invoke/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,17 +151,19 @@ def do_cli( # pylint: disable=R0914
context.function_name, event=event_data, stdout=context.stdout, stderr=context.stderr
)

except FunctionNotFound:
raise UserException("Function {} not found in template".format(function_identifier))
except FunctionNotFound as ex:
raise UserException(
"Function {} not found in template".format(function_identifier), wrapped_from=ex.__class__.__name__
)
except (
InvalidSamDocumentException,
OverridesNotWellDefinedError,
InvalidLayerReference,
DebuggingNotSupported,
) as ex:
raise UserException(str(ex))
raise UserException(str(ex), wrapped_from=ex.__class__.__name__)
except DockerImagePullFailedException as ex:
raise UserException(str(ex))
raise UserException(str(ex), wrapped_from=ex.__class__.__name__)


def _get_event(event_file_name):
Expand Down
8 changes: 5 additions & 3 deletions samcli/commands/local/start_api/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,12 +145,14 @@ def do_cli( # pylint: disable=R0914
service = LocalApiService(lambda_invoke_context=invoke_context, port=port, host=host, static_dir=static_dir)
service.start()

except NoApisDefined:
raise UserException("Template does not have any APIs connected to Lambda functions")
except NoApisDefined as ex:
raise UserException(
"Template does not have any APIs connected to Lambda functions", wrapped_from=ex.__class__.__name__
)
except (
InvalidSamDocumentException,
OverridesNotWellDefinedError,
InvalidLayerReference,
DebuggingNotSupported,
) as ex:
raise UserException(str(ex))
raise UserException(str(ex), wrapped_from=ex.__class__.__name__)
2 changes: 1 addition & 1 deletion samcli/commands/local/start_lambda/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,4 +159,4 @@ def do_cli( # pylint: disable=R0914
InvalidLayerReference,
DebuggingNotSupported,
) as ex:
raise UserException(str(ex))
raise UserException(str(ex), wrapped_from=ex.__class__.__name__)
8 changes: 6 additions & 2 deletions samcli/commands/logs/logs_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@
LOG = logging.getLogger(__name__)


class InvalidTimestampError(UserException):
pass


class LogsCommandContext:
"""
Sets up a context to run the Logs command by parsing the CLI arguments and creating necessary objects to be able
Expand Down Expand Up @@ -213,7 +217,7 @@ def _parse_time(time_str, property_name):

parsed = parse_date(time_str)
if not parsed:
raise UserException("Unable to parse the time provided by '{}'".format(property_name))
raise InvalidTimestampError("Unable to parse the time provided by '{}'".format(property_name))

return to_utc(parsed)

Expand Down Expand Up @@ -267,4 +271,4 @@ def _get_resource_id_from_stack(cfn_client, stack_name, logical_id):
)

# The exception message already has a well formatted error message that we can surface to user
raise UserException(str(ex))
raise UserException(str(ex), wrapped_from=ex.response["Error"]["Code"])
13 changes: 7 additions & 6 deletions samcli/commands/publish/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

from samcli.cli.main import pass_context, common_options as cli_framework_options, aws_creds_options
from samcli.commands._utils.options import template_common_option
from samcli.commands._utils.template import get_template_data
from samcli.commands._utils.template import get_template_data, TemplateFailedParsingException, TemplateNotFoundException
from samcli.lib.telemetry.metrics import track_command
from samcli.cli.cli_config_file import configuration_option, TomlProvider

Expand Down Expand Up @@ -65,9 +65,9 @@ def do_cli(ctx, template, semantic_version):

try:
template_data = get_template_data(template)
except ValueError as ex:
except (TemplateFailedParsingException, TemplateNotFoundException) as ex:
click.secho("Publish Failed", fg="red")
raise UserException(str(ex))
raise ex

# Override SemanticVersion in template metadata when provided in command input
if semantic_version and SERVERLESS_REPO_APPLICATION in template_data.get(METADATA, {}):
Expand All @@ -77,17 +77,18 @@ def do_cli(ctx, template, semantic_version):
publish_output = publish_application(template_data)
click.secho("Publish Succeeded", fg="green")
click.secho(_gen_success_message(publish_output))
except InvalidS3UriError:
except InvalidS3UriError as ex:
click.secho("Publish Failed", fg="red")
raise UserException(
"Your SAM template contains invalid S3 URIs. Please make sure that you have uploaded application "
"artifacts to S3 by packaging the template. See more details in {}".format(SAM_PACKAGE_DOC)
"artifacts to S3 by packaging the template. See more details in {}".format(SAM_PACKAGE_DOC),
wrapped_from=ex.__class__.__name__,
)
except ServerlessRepoError as ex:
click.secho("Publish Failed", fg="red")
LOG.debug("Failed to publish application to serverlessrepo", exc_info=True)
error_msg = "{}\nPlease follow the instructions in {}".format(str(ex), SAM_PUBLISH_DOC)
raise UserException(error_msg)
raise UserException(error_msg, wrapped_from=ex.__class__.__name__)

application_id = publish_output.get("application_id")
_print_console_link(ctx.region, application_id)
Expand Down
4 changes: 3 additions & 1 deletion samcli/commands/validate/validate.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ def do_cli(ctx, template):
click.secho("Template provided at '{}' was invalid SAM Template.".format(template), bg="red")
raise InvalidSamTemplateException(str(e))
except NoCredentialsError as e:
raise UserException("AWS Credentials are required. Please configure your credentials.")
raise UserException(
"AWS Credentials are required. Please configure your credentials.", wrapped_from=e.__class__.__name__
)

click.secho("{} is a valid SAM Template".format(template), fg="green")

Expand Down
5 changes: 4 additions & 1 deletion samcli/lib/telemetry/metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,10 @@ def wrapped(*args, **kwargs):
# Capture exception information and re-raise it later so we can first send metrics.
exception = ex
exit_code = ex.exit_code
exit_reason = type(ex).__name__
if ex.wrapped_from is None:
exit_reason = type(ex).__name__
else:
exit_reason = ex.wrapped_from

except Exception as ex:
exception = ex
Expand Down
6 changes: 4 additions & 2 deletions tests/unit/commands/_utils/test_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,16 @@
_update_relative_paths,
move_template,
get_template_parameters,
TemplateNotFoundException,
TemplateFailedParsingException,
)


class Test_get_template_data(TestCase):
def test_must_raise_if_file_does_not_exist(self):
filename = "filename"

with self.assertRaises(ValueError) as exception_ctx:
with self.assertRaises(TemplateNotFoundException) as exception_ctx:
get_template_data(filename)

ex = exception_ctx.exception
Expand Down Expand Up @@ -80,7 +82,7 @@ def test_must_raise_on_parse_errors(self, exception, pathlib_mock, yaml_parse_mo

with patch("samcli.commands._utils.template.open", m):

with self.assertRaises(ValueError) as ex_ctx:
with self.assertRaises(TemplateFailedParsingException) as ex_ctx:
get_template_data(filename)

actual_exception = ex_ctx.exception
Expand Down
11 changes: 6 additions & 5 deletions tests/unit/commands/init/test_cli.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import botocore.exceptions

from unittest import TestCase
from unittest.mock import patch, ANY

import botocore.exceptions
import click
from click.testing import CliRunner

from samcli.commands.init.init_templates import InitTemplates
Expand Down Expand Up @@ -107,7 +108,7 @@ def test_init_fails_invalid_dep_mgr(self, sd_mock):
def test_init_cli_missing_params_fails(self):
# WHEN we call init without necessary parameters
# THEN we should receive a UserException
with self.assertRaises(UserException):
with self.assertRaises(click.UsageError):
init_cli(
self.ctx,
no_interactive=True,
Expand All @@ -125,7 +126,7 @@ def test_init_cli_missing_params_fails(self):
def test_init_cli_mutually_exclusive_params_fails(self):
# WHEN we call init without necessary parameters
# THEN we should receive a UserException
with self.assertRaises(UserException):
with self.assertRaises(click.UsageError):
init_cli(
self.ctx,
no_interactive=self.no_interactive,
Expand Down Expand Up @@ -253,7 +254,7 @@ def test_init_cli_with_extra_context_not_overriding_default_parameter(self, gene
def test_init_cli_with_extra_context_input_as_wrong_json_raises_exception(self):
# GIVEN extra_context as wrong json
# WHEN a sam init is called
with self.assertRaises(UserException):
with self.assertRaises(click.UsageError):
init_cli(
ctx=self.ctx,
no_interactive=self.no_interactive,
Expand Down
Loading

0 comments on commit af124bc

Please sign in to comment.