Handled unhandled exception to show bug report message (aws#4565)

* Handled unhandled exception to show bug report message

* Fix linting issues

* run black reformat

* Update message text and add unit test

* Run black reformat

* add test cases

* Update actual exitcode of UnhandledException to 1 to not break existing behavior

* remove unused import

* address feedback

* Update messaging

* Update Unit Tests GH Action to make sure Bug_report.md exists

* Update bug report

* Update Bug Report template

* fix typo

* Update wording

* update test

.github/ISSUE_TEMPLATE/Bug_report.md

@@ -32,8 +32,14 @@ If you do find an existing Issue, re-open or add a comment to that Issue instead
 
 ### Additional environment details (Ex: Windows, Mac, Amazon Linux etc)
 
+<!-- Either provide the following info (for AWS SAM CLI v1.68.0 or before) or paste the output of `sam --info` (AWS SAM CLI v1.69.0 or after). -->
+
 1. OS:
 2. `sam --version`:
 3. AWS region:
 
+```
+# Paste the output of `sam --info` here
+```
+
 `Add --debug flag to command you are running`

.github/workflows/build.yml

@@ -25,5 +25,6 @@ jobs:
     - uses: actions/setup-python@v4
       with:
         python-version: ${{ matrix.python }}
+    - run: test -f "./.github/ISSUE_TEMPLATE/Bug_report.md"  # prevent Bug_report.md from being renamed or deleted
     - run: make init
     - run: make pr

samcli/commands/exceptions.py

@@ -1,6 +1,9 @@
 """
 Class containing error conditions that are exposed to the user.
 """
+import traceback
+from urllib.parse import quote
+from typing import Optional, IO
 
 import click
 
@@ -25,6 +28,45 @@ def __init__(self, message, wrapped_from=None):
         click.ClickException.__init__(self, message)
 
 
+class UnhandledException(click.ClickException):
+    """
+    Exception class to re-wrap any exception that is not a UserException.
+    Typically this means there is a bug in SAM CLI.
+    """
+
+    GH_ISSUE_SEARCH_URL = "https://github.com/aws/aws-sam-cli/issues?q=is%3Aissue+is%3Aopen+{title}"
+    GH_BUG_REPORT_URL = "https://github.com/aws/aws-sam-cli/issues/new?template=Bug_report.md&title={title}"
+    # NOTE (hawflau): actual exitcode is 1 to not break existing behavior. Only report 255 to telemetry
+    exit_code = 1
+
+    def __init__(self, command: str, exception: Exception) -> None:
+        self._command = command
+        self._exception = exception
+        self.__traceback__ = self._exception.__traceback__
+
+        click.ClickException.__init__(self, type(exception).__name__)
+
+    def show(self, file: Optional[IO] = None) -> None:
+        """Overriding show to customize printing stack trace and message"""
+        if file is None:
+            file = click._compat.get_text_stderr()  # pylint: disable=protected-access
+
+        tb = "".join(traceback.format_tb(self.__traceback__))
+        click.echo(f"\nTraceback:\n{tb}", file=file, err=True)
+
+        encoded_title = quote(f"Bug: {self._command} - {type(self._exception).__name__}")
+        lookup_url = self.GH_ISSUE_SEARCH_URL.format(title=encoded_title)
+        create_issue_url = self.GH_BUG_REPORT_URL.format(title=encoded_title)
+        msg = (
+            f'An unexpected error was encountered while executing "{self._command}".\n'
+            "Search for an existing issue:\n"
+            f"{lookup_url}\n"
+            "Or create a bug report:\n"
+            f"{create_issue_url}"
+        )
+        click.secho(msg, file=file, err=True, fg="yellow")
+
+
 class CredentialsError(UserException):
     """
     Exception class when credentials that have been passed are invalid.

samcli/lib/telemetry/metric.py

@@ -18,7 +18,7 @@
 from samcli.cli.context import Context
 from samcli.cli.global_config import GlobalConfig
 from samcli.commands._utils.experimental import get_all_experimental_statues
-from samcli.commands.exceptions import UserException
+from samcli.commands.exceptions import UserException, UnhandledException
 from samcli.lib.hook.hook_config import HookPackageConfig
 from samcli.lib.hook.hook_wrapper import INTERNAL_PACKAGES_ROOT
 from samcli.lib.hook.exceptions import InvalidHookPackageConfigException
@@ -148,16 +148,24 @@ def wrapped(*args, **kwargs):
             # Execute the function and capture return value. This is returned back by the wrapper
             # First argument of all commands should be the Context
             return_value = func(*args, **kwargs)
-        except UserException as ex:
+        except (
+            UserException,
+            click.BadOptionUsage,
+            click.BadArgumentUsage,
+            click.BadParameter,
+            click.UsageError,
+        ) as ex:
             # Capture exception information and re-raise it later so we can first send metrics.
             exception = ex
-            exit_code = ex.exit_code
-            if ex.wrapped_from is None:
-                exit_reason = type(ex).__name__
-            else:
+            # TODO (hawflau): review exitcode meaning. We now understand exitcode 1 as errors fixable by users.
+            exit_code = 1
+            if hasattr(ex, "wrapped_from") and ex.wrapped_from:
                 exit_reason = ex.wrapped_from
+            else:
+                exit_reason = type(ex).__name__
         except Exception as ex:
-            exception = ex
+            command = ctx.command_path if ctx else ""
+            exception = UnhandledException(command, ex)
             # Standard Unix practice to return exit code 255 on fatal/unhandled exit.
             exit_code = 255
             exit_reason = type(ex).__name__

tests/unit/commands/test_exceptions.py

@@ -0,0 +1,33 @@
+import io
+
+from unittest import TestCase
+from unittest.mock import patch, Mock
+
+
+from samcli.commands.exceptions import UnhandledException
+
+
+class TestUnhandledException(TestCase):
+    def test_show_must_print_traceback_and_message(self):
+        wrapped_exception = None
+        try:
+            raise Exception("my_exception")
+        except Exception as e:
+            wrapped_exception = e
+
+        unhandled_exception = UnhandledException("test_command", wrapped_exception)
+        f = io.StringIO()
+        unhandled_exception.show(f)
+
+        output = f.getvalue()
+
+        self.assertIn("Traceback:", output)
+        self.assertIn('raise Exception("my_exception")', output)
+        self.assertIn(
+            'An unexpected error was encountered while executing "test_command".\n'
+            "Search for an existing issue:\n"
+            "https://github.com/aws/aws-sam-cli/issues?q=is%3Aissue+is%3Aopen+Bug%3A%20test_command%20-%20Exception\n"
+            "Or create a bug report:\n"
+            "https://github.com/aws/aws-sam-cli/issues/new?template=Bug_report.md&title=Bug%3A%20test_command%20-%20Exception",
+            output,
+        )

tests/unit/lib/telemetry/test_metric.py

@@ -1,14 +1,12 @@
 import platform
 import time
 
-from parameterized import parameterized
-
-import samcli
-
 from unittest import TestCase
 from unittest.mock import patch, Mock, ANY, call
 
 import pytest
+import click
+from parameterized import parameterized
 
 from samcli.lib.hook.exceptions import InvalidHookPackageConfigException, InvalidHookWrapperException
 from samcli.lib.iac.plugins_interfaces import ProjectTypes
@@ -28,7 +26,7 @@
     ProjectDetails,
     _send_command_run_metrics,
 )
-from samcli.commands.exceptions import UserException
+from samcli.commands.exceptions import UserException, UnhandledException
 
 
 class TestSendInstalledMetric(TestCase):
@@ -184,6 +182,10 @@ def real_fn():
             (KeyError("IO Error test"), "KeyError", 255),
             (UserException("Something went wrong", wrapped_from="CustomException"), "CustomException", 1),
             (UserException("Something went wrong"), "UserException", 1),
+            (click.BadOptionUsage("--my-opt", "Wrong option usage"), "BadOptionUsage", 1),
+            (click.BadArgumentUsage("Wrong argument usage"), "BadArgumentUsage", 1),
+            (click.BadParameter("Bad param"), "BadParameter", 1),
+            (click.UsageError("UsageError"), "UsageError", 1),
         ]
     )
     @patch("samcli.lib.telemetry.metric.Context")
@@ -194,12 +196,13 @@ def test_records_exceptions(self, exception, expected_exception, expected_code,
         def real_fn():
             raise exception
 
-        with self.assertRaises(exception.__class__) as context:
+        expected_exception_class = exception.__class__ if expected_code != 255 else UnhandledException
+        with self.assertRaises(expected_exception_class) as context:
             track_command(real_fn)()
             self.assertEqual(
                 context.exception,
                 exception,
-                "Must re-raise the original exception object " "without modification",
+                "Must re-raise the original exception object without modification",
             )
 
         run_metrics_mock.assert_called_with(self.context_mock, ANY, expected_exception, expected_code)
@@ -257,21 +260,29 @@ def func(**kwargs):
             (KeyError, 255),
             (UserException, 1),
             (InvalidHookWrapperException, 1),
+            (click.BadOptionUsage, 1),
+            (click.BadArgumentUsage, 1),
+            (click.BadParameter, 1),
+            (click.UsageError, 1),
         ]
     )
     @patch("samcli.lib.telemetry.metric.Context")
     @patch("samcli.lib.telemetry.metric._send_command_run_metrics")
-    def test_context_contains_exception(self, expected_exception, expected_code, run_metrics_mock, context_mock):
-        self.context_mock.exception = expected_exception("some exception")
+    def test_context_contains_exception(self, exception, exitcode, run_metrics_mock, context_mock):
+        if exception == click.BadOptionUsage:
+            self.context_mock.exception = exception("--opt", "some exception")
+        else:
+            self.context_mock.exception = exception("some exception")
         context_mock.get_current_context.return_value = self.context_mock
 
         mocked_func = Mock()
 
+        expected_exception = exception if exitcode == 1 else UnhandledException
         with self.assertRaises(expected_exception):
             track_command(mocked_func)()
 
         mocked_func.assert_not_called()
-        run_metrics_mock.assert_called_with(self.context_mock, ANY, expected_exception.__name__, expected_code)
+        run_metrics_mock.assert_called_with(self.context_mock, ANY, exception.__name__, exitcode)
 
 
 class TestSendCommandMetrics(TestCase):