Error handling

jsonschema provides a rich error tree of info, by parsing each error
we can pull out relevant info and re-write the error messages.

This covers current error handling behaviour.

This includes new error handling behaviour for types and formatting of
the ports field.

Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>

compose/config/config.py

@@ -18,6 +18,8 @@ from .errors import (
 )
 
 
+VALID_NAME_CHARS = '[a-zA-Z0-9\._\-]'
+
 DOCKER_CONFIG_KEYS = [
     'cap_add',
     'cap_drop',
@@ -155,6 +157,77 @@ def format_ports(instance):
     return False
 
 
+def get_unsupported_config_msg(service_name, error_key):
+    msg = "Unsupported config option for '{}' service: '{}'".format(service_name, error_key)
+    if error_key in DOCKER_CONFIG_HINTS:
+        msg += " (did you mean '{}'?)".format(DOCKER_CONFIG_HINTS[error_key])
+    return msg
+
+
+def process_errors(errors):
+    """
+    jsonschema gives us an error tree full of information to explain what has
+    gone wrong. Process each error and pull out relevant information and re-write
+    helpful error messages that are relevant.
+    """
+    def _parse_key_from_error_msg(error):
+        return error.message.split("'")[1]
+
+    root_msgs = []
+    invalid_keys = []
+    required = []
+    type_errors = []
+
+    for error in errors:
+        # handle root level errors
+        if len(error.path) == 0:
+            if error.validator == 'type':
+                msg = "Top level object needs to be a dictionary. Check your .yml file that you have defined a service at the top level."
+                root_msgs.append(msg)
+            elif error.validator == 'additionalProperties':
+                invalid_service_name = _parse_key_from_error_msg(error)
+                msg = "Invalid service name '{}' - only {} characters are allowed".format(invalid_service_name, VALID_NAME_CHARS)
+                root_msgs.append(msg)
+            else:
+                root_msgs.append(error.message)
+
+        else:
+            # handle service level errors
+            service_name = error.path[0]
+
+            if error.validator == 'additionalProperties':
+                invalid_config_key = _parse_key_from_error_msg(error)
+                invalid_keys.append(get_unsupported_config_msg(service_name, invalid_config_key))
+            elif error.validator == 'anyOf':
+                if 'image' in error.instance and 'build' in error.instance:
+                    required.append("Service '{}' has both an image and build path specified. A service can either be built to image or use an existing image, not both.".format(service_name))
+                elif 'image' not in error.instance and 'build' not in error.instance:
+                    required.append("Service '{}' has neither an image nor a build path specified. Exactly one must be provided.".format(service_name))
+                else:
+                    required.append(error.message)
+            elif error.validator == 'type':
+                msg = "a"
+                if error.validator_value == "array":
+                    msg = "an"
+
+                try:
+                    config_key = error.path[1]
+                    type_errors.append("Service '{}' has an invalid value for '{}', it should be {} {}".format(service_name, config_key, msg, error.validator_value))
+                except IndexError:
+                    config_key = error.path[0]
+                    root_msgs.append("Service '{}' doesn\'t have any configuration options. All top level keys in your docker-compose.yml must map to a dictionary of configuration options.'".format(config_key))
+            elif error.validator == 'required':
+                config_key = error.path[1]
+                required.append("Service '{}' option '{}' is invalid, {}".format(service_name, config_key, error.message))
+            elif error.validator == 'dependencies':
+                dependency_key = error.validator_value.keys()[0]
+                required_keys = ",".join(error.validator_value[dependency_key])
+                required.append("Invalid '{}' configuration for '{}' service: when defining '{}' you must set '{}' as well".format(
+                    dependency_key, service_name, dependency_key, required_keys))
+
+    return "\n".join(root_msgs + invalid_keys + required + type_errors)
+
+
 def validate_against_schema(config):
     config_source_dir = os.path.dirname(os.path.abspath(__file__))
     schema_file = os.path.join(config_source_dir, "schema.json")
@@ -164,9 +237,10 @@ def validate_against_schema(config):
 
     validation_output = Draft4Validator(schema, format_checker=FormatChecker(["ports"]))
 
-    errors = [error.message for error in sorted(validation_output.iter_errors(config), key=str)]
+    errors = [error for error in sorted(validation_output.iter_errors(config), key=str)]
     if errors:
-        raise ConfigurationError("Validation failed, reason(s): {}".format("\n".join(errors)))
+        error_msg = process_errors(errors)
+        raise ConfigurationError("Validation failed, reason(s):\n{}".format(error_msg))
 
 
 def load(config_details):

compose/service.py

@@ -12,7 +12,7 @@ from docker.errors import APIError
 from docker.utils import create_host_config, LogConfig
 
 from . import __version__
-from .config import DOCKER_CONFIG_KEYS, merge_environment
+from .config import DOCKER_CONFIG_KEYS, merge_environment, VALID_NAME_CHARS
 from .const import (
     DEFAULT_TIMEOUT,
     LABEL_CONTAINER_NUMBER,
@@ -49,8 +49,6 @@ DOCKER_START_KEYS = [
     'security_opt',
 ]
 
-VALID_NAME_CHARS = '[a-zA-Z0-9\._\-]'
-
 
 class BuildError(Exception):
     def __init__(self, service, reason):

tests/unit/config_test.py

@@ -371,7 +371,8 @@ class MemoryOptionsTest(unittest.TestCase):
         When you set a 'memswap_limit' it is invalid config unless you also set
         a mem_limit
         """
-        with self.assertRaisesRegexp(config.ConfigurationError, "u'mem_limit' is a dependency of u'memswap_limit'"):
+        expected_error_msg = "Invalid 'memswap_limit' configuration for 'foo' service: when defining 'memswap_limit' you must set 'mem_limit' as well"
+        with self.assertRaisesRegexp(config.ConfigurationError, expected_error_msg):
             config.load(
                 config.ConfigDetails(
                     {
@@ -625,7 +626,8 @@ class ExtendsTest(unittest.TestCase):
             )
 
     def test_extends_validation_invalid_key(self):
-        with self.assertRaisesRegexp(config.ConfigurationError, "'rogue_key' was unexpected"):
+        expected_error_msg = "Unsupported config option for 'web' service: 'rogue_key'"
+        with self.assertRaisesRegexp(config.ConfigurationError, expected_error_msg):
             config.load(
                 config.ConfigDetails(
                     {