fix: better error messages on build failures (#146)

* fix: better error messages on build failures

Why is this change necessary?

* Generic "Binary Validation Failed" does not add enough value

How does it address the issue?

* Showcases which binary has failed and shows which paths were looked
at.

What side effects does this change have?

* None.

* fix: explicit error messages on binary validation failure

- fail workflow explicitly if binary resolution failed

* fix: surface error messages for all binary validation failures

* fix: fix tests for unittest imports

* fix: make pylint happy

* fix: appveyor black version

.appveyor.yml

@@ -101,7 +101,7 @@ for:
       - sh: "PATH=/opt/gradle/gradle-5.5/bin:$PATH"
 
       # Install black
-      - sh: "wget -O /tmp/black https://github.com/python/black/releases/download/19.3b0/black"
+      - sh: "wget -O /tmp/black https://github.com/python/black/releases/download/19.10b0/black"
       - sh: "chmod +x /tmp/black"
       - sh: "/tmp/black --version"
 

.gitignore

@@ -257,7 +257,7 @@ wheels/
 # Installer logs
 pip-log.txt
 pip-delete-this-directory.txt
-
+pip-wheel-metadata/
 # Unit test / coverage reports
 htmlcov/
 .tox/
@@ -389,4 +389,4 @@ $RECYCLE.BIN/
 
 tests/integration/workflows/go_dep/data/src/*/vendor/*
 
-# End of https://www.gitignore.io/api/osx,node,macos,linux,python,windows,pycharm,intellij,sublimetext,visualstudiocode
+# End of https://www.gitignore.io/api/osx,node,macos,linux,python,windows,pycharm,intellij,sublimetext,visualstudiocode

aws_lambda_builders/workflow.py

@@ -40,25 +40,39 @@ def sanitize(func):
 
     @functools.wraps(func)
     def wrapper(self, *args, **kwargs):
-        valid_paths = []
+        valid_paths = {}
+        invalid_paths = {}
         # NOTE: we need to access binaries to get paths and resolvers, before validating.
-        binaries_copy = self.binaries
-        for binary, binary_path in binaries_copy.items():
-            validator = binary_path.validator
-            exec_paths = binary_path.resolver.exec_paths if not binary_path.path_provided else binary_path.binary_path
+        for binary, binary_checker in self.binaries.items():
+            invalid_paths[binary] = []
+            try:
+                exec_paths = (
+                    binary_checker.resolver.exec_paths
+                    if not binary_checker.path_provided
+                    else binary_checker.binary_path
+                )
+            except ValueError as ex:
+                raise WorkflowFailedError(workflow_name=self.NAME, action_name="Resolver", reason=str(ex))
             for executable_path in exec_paths:
-                valid_path = None
                 try:
-                    valid_path = validator.validate(executable_path)
+                    valid_path = binary_checker.validator.validate(executable_path)
+                    if valid_path:
+                        valid_paths[binary] = valid_path
                 except MisMatchRuntimeError as ex:
                     LOG.debug("Invalid executable for %s at %s", binary, executable_path, exc_info=str(ex))
-                if valid_path:
-                    binary_path.binary_path = valid_path
-                    valid_paths.append(valid_path)
+                    invalid_paths[binary].append(executable_path)
+                if valid_paths.get(binary, None):
+                    binary_checker.binary_path = valid_paths[binary]
                     break
-        self.binaries = binaries_copy
         if len(self.binaries) != len(valid_paths):
-            raise WorkflowFailedError(workflow_name=self.NAME, action_name=None, reason="Binary validation failed!")
+            validation_failed_binaries = set(self.binaries.keys()).difference(valid_paths.keys())
+            messages = []
+            for validation_failed_binary in validation_failed_binaries:
+                message = "Binary validation failed for {0}, searched for {0} in following locations  : {1} which did not satisfy constraints for runtime: {2}. Do you have {0} for runtime: {2} on your PATH?".format(
+                    validation_failed_binary, invalid_paths[validation_failed_binary], self.runtime
+                )
+                messages.append(message)
+            raise WorkflowFailedError(workflow_name=self.NAME, action_name="Validation", reason="\n".join(messages))
         func(self, *args, **kwargs)
 
     return wrapper
@@ -261,6 +275,7 @@ def run(self):
 
                 raise WorkflowFailedError(workflow_name=self.NAME, action_name=action.NAME, reason=str(ex))
             except Exception as ex:
+
                 LOG.debug("%s raised unhandled exception", action_info, exc_info=ex)
 
                 raise WorkflowUnknownError(workflow_name=self.NAME, action_name=action.NAME, reason=str(ex))

aws_lambda_builders/workflows/python_pip/actions.py

@@ -4,6 +4,7 @@
 
 from aws_lambda_builders.actions import BaseAction, Purpose, ActionFailedError
 from aws_lambda_builders.workflows.python_pip.utils import OSUtils
+from .exceptions import MissingPipError
 from .packager import PythonPipDependencyBuilder, PackagerError, DependencyBuilder, SubprocessPip, PipRunner
 
 
@@ -24,7 +25,10 @@ def __init__(self, artifacts_dir, scratch_dir, manifest_path, runtime, binaries)
     def execute(self):
         os_utils = OSUtils()
         python_path = self.binaries[self.LANGUAGE].binary_path
-        pip = SubprocessPip(osutils=os_utils, python_exe=python_path)
+        try:
+            pip = SubprocessPip(osutils=os_utils, python_exe=python_path)
+        except MissingPipError as ex:
+            raise ActionFailedError(str(ex))
         pip_runner = PipRunner(python_exe=python_path, pip=pip)
         dependency_builder = DependencyBuilder(osutils=os_utils, pip_runner=pip_runner, runtime=self.runtime)
 

aws_lambda_builders/workflows/python_pip/compat.py

@@ -1,5 +1,6 @@
 import os
 
+from aws_lambda_builders.workflows.python_pip.exceptions import MissingPipError
 from aws_lambda_builders.workflows.python_pip.utils import OSUtils
 
 
@@ -8,6 +9,8 @@ def pip_import_string(python_exe):
     cmd = [python_exe, "-c", "import pip; print(pip.__version__)"]
     p = os_utils.popen(cmd, stdout=os_utils.pipe, stderr=os_utils.pipe)
     stdout, stderr = p.communicate()
+    if not p.returncode == 0:
+        raise MissingPipError(python_path=python_exe)
     pip_version = stdout.decode("utf-8").strip()
     pip_major_version = int(pip_version.split(".")[0])
     pip_minor_version = int(pip_version.split(".")[1])

aws_lambda_builders/workflows/python_pip/exceptions.py

@@ -0,0 +1,8 @@
+"""
+Python pip specific workflow exceptions.
+"""
+from aws_lambda_builders.exceptions import LambdaBuilderError
+
+
+class MissingPipError(LambdaBuilderError):
+    MESSAGE = "pip executable not found in your python environment at {python_path}"

tests/functional/testdata/cwd.py

@@ -1,3 +1,3 @@
 import os
 
-print (os.getcwd())
+print(os.getcwd())

tests/integration/workflows/go_modules/test_go.py

@@ -37,7 +37,7 @@ def test_builds_project_without_dependencies(self):
         )
         expected_files = {"no-deps-main"}
         output_files = set(os.listdir(self.artifacts_dir))
-        print (output_files)
+        print(output_files)
         self.assertEquals(expected_files, output_files)
 
     def test_builds_project_with_dependencies(self):

tests/integration/workflows/python_pip/test_python_pip.py

@@ -64,7 +64,7 @@ def test_mismatch_runtime_python_project(self):
                 runtime=self.runtime_mismatch[self.runtime],
             )
         except WorkflowFailedError as ex:
-            self.assertIn("Binary validation failed!", str(ex))
+            self.assertIn("Binary validation failed", str(ex))
 
     def test_runtime_validate_python_project_fail_open_unsupported_runtime(self):
         with self.assertRaises(WorkflowFailedError):

tests/unit/test_workflow.py

@@ -1,7 +1,8 @@
 import os
 import sys
 from unittest import TestCase
-from mock import Mock, call
+
+from mock import Mock, MagicMock, call
 
 try:
     import pathlib
@@ -12,7 +13,7 @@
 from aws_lambda_builders.validator import RuntimeValidator
 from aws_lambda_builders.workflow import BaseWorkflow, Capability
 from aws_lambda_builders.registry import get_workflow, DEFAULT_REGISTRY
-from aws_lambda_builders.exceptions import WorkflowFailedError, WorkflowUnknownError
+from aws_lambda_builders.exceptions import WorkflowFailedError, WorkflowUnknownError, MisMatchRuntimeError
 from aws_lambda_builders.actions import ActionFailedError
 
 
@@ -206,6 +207,42 @@ def test_must_execute_actions_in_sequence(self):
         )
         self.assertTrue(validator_mock.validate.call_count, 1)
 
+    def test_must_fail_workflow_binary_resolution_failure(self):
+        action_mock = Mock()
+        validator_mock = Mock()
+        validator_mock.validate = Mock()
+        validator_mock.validate.return_value = None
+        resolver_mock = Mock()
+        resolver_mock.exec_paths = MagicMock(side_effect=ValueError("Binary could not be resolved"))
+        binaries_mock = Mock()
+        binaries_mock.return_value = []
+
+        self.work.get_validators = lambda: validator_mock
+        self.work.get_resolvers = lambda: resolver_mock
+        self.work.actions = [action_mock.action1, action_mock.action2, action_mock.action3]
+        self.work.binaries = {"binary": BinaryPath(resolver=resolver_mock, validator=validator_mock, binary="binary")}
+        with self.assertRaises(WorkflowFailedError) as ex:
+            self.work.run()
+
+    def test_must_fail_workflow_binary_validation_failure(self):
+        action_mock = Mock()
+        validator_mock = Mock()
+        validator_mock.validate = Mock()
+        validator_mock.validate = MagicMock(
+            side_effect=MisMatchRuntimeError(language="test", required_runtime="test1", runtime_path="/usr/bin/binary")
+        )
+        resolver_mock = Mock()
+        resolver_mock.exec_paths = ["/usr/bin/binary"]
+        binaries_mock = Mock()
+        binaries_mock.return_value = []
+
+        self.work.get_validators = lambda: validator_mock
+        self.work.get_resolvers = lambda: resolver_mock
+        self.work.actions = [action_mock.action1, action_mock.action2, action_mock.action3]
+        self.work.binaries = {"binary": BinaryPath(resolver=resolver_mock, validator=validator_mock, binary="binary")}
+        with self.assertRaises(WorkflowFailedError) as ex:
+            self.work.run()
+
     def test_must_raise_with_no_actions(self):
         self.work.actions = []
 

tests/unit/workflows/python_pip/test_actions.py

@@ -7,6 +7,7 @@
 from aws_lambda_builders.binary_path import BinaryPath
 
 from aws_lambda_builders.workflows.python_pip.actions import PythonPipBuildAction
+from aws_lambda_builders.workflows.python_pip.exceptions import MissingPipError
 from aws_lambda_builders.workflows.python_pip.packager import PackagerError
 
 
@@ -43,3 +44,18 @@ def test_must_raise_exception_on_failure(self, PythonPipDependencyBuilderMock):
 
         with self.assertRaises(ActionFailedError):
             action.execute()
+
+    @patch("aws_lambda_builders.workflows.python_pip.actions.SubprocessPip")
+    def test_must_raise_exception_on_pip_failure(self, PythonSubProcessPipMock):
+        PythonSubProcessPipMock.side_effect = MissingPipError(python_path="mockpath")
+
+        action = PythonPipBuildAction(
+            "artifacts",
+            "scratch_dir",
+            "manifest",
+            "runtime",
+            {"python": BinaryPath(resolver=Mock(), validator=Mock(), binary="python", binary_path=sys.executable)},
+        )
+
+        with self.assertRaises(ActionFailedError):
+            action.execute()

tests/unit/workflows/python_pip/test_packager.py

@@ -20,7 +20,7 @@
 
 from aws_lambda_builders.workflows.python_pip import packager
 
-print (packager)
+print(packager)
 
 FakePipCall = namedtuple("FakePipEntry", ["args", "env_vars", "shim"])