Improve handling of connection errors and error messages.

Signed-off-by: Daniel Nephin <dnephin@docker.com>

compose/cli/command.py

@@ -1,52 +1,25 @@
 from __future__ import absolute_import
 from __future__ import unicode_literals
 
-import contextlib
 import logging
 import os
 import re
 
 import six
-from requests.exceptions import ConnectionError
-from requests.exceptions import SSLError
 
-from . import errors
 from . import verbose_proxy
 from .. import config
 from ..const import API_VERSIONS
 from ..project import Project
 from .docker_client import docker_client
-from .utils import call_silently
 from .utils import get_version_info
-from .utils import is_mac
-from .utils import is_ubuntu
 
 log = logging.getLogger(__name__)
 
 
-@contextlib.contextmanager
+def project_from_options(project_dir, options):
-def friendly_error_message():
-    try:
-        yield
-    except SSLError as e:
-        raise errors.UserError('SSL error: %s' % e)
-    except ConnectionError:
-        if call_silently(['which', 'docker']) != 0:
-            if is_mac():
-                raise errors.DockerNotFoundMac()
-            elif is_ubuntu():
-                raise errors.DockerNotFoundUbuntu()
-            else:
-                raise errors.DockerNotFoundGeneric()
-        elif call_silently(['which', 'docker-machine']) == 0:
-            raise errors.ConnectionErrorDockerMachine()
-        else:
-            raise errors.ConnectionErrorGeneric(get_client().base_url)
-
-
-def project_from_options(base_dir, options):
     return get_project(
-        base_dir,
+        project_dir,
         get_config_path_from_options(options),
         project_name=options.get('--project-name'),
         verbose=options.get('--verbose'),
@@ -76,8 +49,8 @@ def get_client(verbose=False, version=None):
     return client
 
 
-def get_project(base_dir, config_path=None, project_name=None, verbose=False):
+def get_project(project_dir, config_path=None, project_name=None, verbose=False):
-    config_details = config.find(base_dir, config_path)
+    config_details = config.find(project_dir, config_path)
     project_name = get_project_name(config_details.working_dir, project_name)
     config_data = config.load(config_details)
 

compose/cli/errors.py

@@ -1,10 +1,27 @@
 from __future__ import absolute_import
 from __future__ import unicode_literals
 
+import contextlib
+import logging
 from textwrap import dedent
 
+from docker.errors import APIError
+from requests.exceptions import ConnectionError as RequestsConnectionError
+from requests.exceptions import ReadTimeout
+from requests.exceptions import SSLError
+
+from ..const import API_VERSION_TO_ENGINE_VERSION
+from ..const import HTTP_TIMEOUT
+from .utils import call_silently
+from .utils import is_mac
+from .utils import is_ubuntu
+
+
+log = logging.getLogger(__name__)
+
 
 class UserError(Exception):
+
     def __init__(self, msg):
         self.msg = dedent(msg).strip()
 
@@ -14,44 +31,90 @@ class UserError(Exception):
     __str__ = __unicode__
 
 
-class DockerNotFoundMac(UserError):
+class ConnectionError(Exception):
-    def __init__(self):
+    pass
-        super(DockerNotFoundMac, self).__init__("""
-        Couldn't connect to Docker daemon. You might need to install docker-osx:
-
-        https://github.com/noplay/docker-osx
-        """)
 
 
-class DockerNotFoundUbuntu(UserError):
+@contextlib.contextmanager
-    def __init__(self):
+def handle_connection_errors(client):
-        super(DockerNotFoundUbuntu, self).__init__("""
+    try:
-        Couldn't connect to Docker daemon. You might need to install Docker:
+        yield
-
+    except SSLError as e:
-        https://docs.docker.com/engine/installation/ubuntulinux/
+        log.error('SSL error: %s' % e)
-        """)
+        raise ConnectionError()
+    except RequestsConnectionError:
+        if call_silently(['which', 'docker']) != 0:
+            if is_mac():
+                exit_with_error(docker_not_found_mac)
+            if is_ubuntu():
+                exit_with_error(docker_not_found_ubuntu)
+            exit_with_error(docker_not_found_generic)
+        if call_silently(['which', 'docker-machine']) == 0:
+            exit_with_error(conn_error_docker_machine)
+        exit_with_error(conn_error_generic.format(url=client.base_url))
+    except APIError as e:
+        log_api_error(e, client.api_version)
+        raise ConnectionError()
+    except ReadTimeout as e:
+        log.error(
+            "An HTTP request took too long to complete. Retry with --verbose to "
+            "obtain debug information.\n"
+            "If you encounter this issue regularly because of slow network "
+            "conditions, consider setting COMPOSE_HTTP_TIMEOUT to a higher "
+            "value (current value: %s)." % HTTP_TIMEOUT)
+        raise ConnectionError()
 
 
-class DockerNotFoundGeneric(UserError):
+def log_api_error(e, client_version):
-    def __init__(self):
+    if 'client is newer than server' not in e.explanation:
-        super(DockerNotFoundGeneric, self).__init__("""
+        log.error(e.explanation)
-        Couldn't connect to Docker daemon. You might need to install Docker:
+        return
 
-        https://docs.docker.com/engine/installation/
+    version = API_VERSION_TO_ENGINE_VERSION.get(client_version)
-        """)
+    if not version:
+        # They've set a custom API version
+        log.error(e.explanation)
+        return
+
+    log.error(
+        "The Docker Engine version is less than the minimum required by "
+        "Compose. Your current project requires a Docker Engine of "
+        "version {version} or greater.".format(version=version))
 
 
-class ConnectionErrorDockerMachine(UserError):
+def exit_with_error(msg):
-    def __init__(self):
+    log.error(dedent(msg).strip())
-        super(ConnectionErrorDockerMachine, self).__init__("""
+    raise ConnectionError()
-        Couldn't connect to Docker daemon - you might need to run `docker-machine start default`.
-        """)
 
 
-class ConnectionErrorGeneric(UserError):
+docker_not_found_mac = """
-    def __init__(self, url):
+    Couldn't connect to Docker daemon. You might need to install docker-osx:
-        super(ConnectionErrorGeneric, self).__init__("""
-        Couldn't connect to Docker daemon at %s - is it running?
 
-        If it's at a non-standard location, specify the URL with the DOCKER_HOST environment variable.
+    https://github.com/noplay/docker-osx
-        """ % url)
+"""
+
+
+docker_not_found_ubuntu = """
+    Couldn't connect to Docker daemon. You might need to install Docker:
+
+    https://docs.docker.com/engine/installation/ubuntulinux/
+"""
+
+
+docker_not_found_generic = """
+    Couldn't connect to Docker daemon. You might need to install Docker:
+
+    https://docs.docker.com/engine/installation/
+"""
+
+
+conn_error_docker_machine = """
+    Couldn't connect to Docker daemon - you might need to run `docker-machine start default`.
+"""
+
+
+conn_error_generic = """
+    Couldn't connect to Docker daemon at {url} - is it running?
+
+    If it's at a non-standard location, specify the URL with the DOCKER_HOST environment variable.
+"""

compose/cli/main.py

@@ -11,18 +11,14 @@ import sys
 from inspect import getdoc
 from operator import attrgetter
 
-from docker.errors import APIError
+from . import errors
-from requests.exceptions import ReadTimeout
-
 from . import signals
 from .. import __version__
 from ..config import config
 from ..config import ConfigurationError
 from ..config import parse_environment
 from ..config.serialize import serialize_config
-from ..const import API_VERSION_TO_ENGINE_VERSION
 from ..const import DEFAULT_TIMEOUT
-from ..const import HTTP_TIMEOUT
 from ..const import IS_WINDOWS_PLATFORM
 from ..progress_stream import StreamOutputError
 from ..project import NoSuchService
@@ -31,7 +27,6 @@ from ..service import BuildError
 from ..service import ConvergenceStrategy
 from ..service import ImageType
 from ..service import NeedsBuildError
-from .command import friendly_error_message
 from .command import get_config_path_from_options
 from .command import project_from_options
 from .docopt_command import DocoptDispatcher
@@ -53,7 +48,6 @@ console_handler = logging.StreamHandler(sys.stderr)
 
 
 def main():
-    setup_logging()
     command = dispatch()
 
     try:
@@ -64,9 +58,6 @@ def main():
     except (UserError, NoSuchService, ConfigurationError) as e:
         log.error(e.msg)
         sys.exit(1)
-    except APIError as e:
-        log_api_error(e)
-        sys.exit(1)
     except BuildError as e:
         log.error("Service '%s' failed to build: %s" % (e.service.name, e.reason))
         sys.exit(1)
@@ -76,18 +67,12 @@ def main():
     except NeedsBuildError as e:
         log.error("Service '%s' needs to be built, but --no-build was passed." % e.service.name)
         sys.exit(1)
-    except ReadTimeout as e:
+    except errors.ConnectionError:
-        log.error(
-            "An HTTP request took too long to complete. Retry with --verbose to "
-            "obtain debug information.\n"
-            "If you encounter this issue regularly because of slow network "
-            "conditions, consider setting COMPOSE_HTTP_TIMEOUT to a higher "
-            "value (current value: %s)." % HTTP_TIMEOUT
-        )
         sys.exit(1)
 
 
 def dispatch():
+    setup_logging()
     dispatcher = DocoptDispatcher(
         TopLevelCommand,
         {'options_first': True, 'version': get_version_info('compose')})
@@ -116,26 +101,10 @@ def perform_command(options, handler, command_options):
 
     project = project_from_options('.', options)
     command = TopLevelCommand(project)
-    with friendly_error_message():
+    with errors.handle_connection_errors(project.client):
         handler(command, command_options)
 
 
-def log_api_error(e):
-    if 'client is newer than server' in e.explanation:
-        # we need JSON formatted errors. In the meantime...
-        # TODO: fix this by refactoring project dispatch
-        # http://github.com/docker/compose/pull/2832#commitcomment-15923800
-        client_version = e.explanation.split('client API version: ')[1].split(',')[0]
-        log.error(
-            "The engine version is lesser than the minimum required by "
-            "compose. Your current project requires a Docker Engine of "
-            "version {version} or superior.".format(
-                version=API_VERSION_TO_ENGINE_VERSION[client_version]
-            ))
-    else:
-        log.error(e.explanation)
-
-
 def setup_logging():
     root_logger = logging.getLogger()
     root_logger.addHandler(console_handler)

tests/unit/cli/command_test.py

@@ -4,28 +4,12 @@ from __future__ import unicode_literals
 import os
 
 import pytest
-from requests.exceptions import ConnectionError
 
-from compose.cli import errors
-from compose.cli.command import friendly_error_message
 from compose.cli.command import get_config_path_from_options
 from compose.const import IS_WINDOWS_PLATFORM
 from tests import mock
 
 
-class TestFriendlyErrorMessage(object):
-
-    def test_dispatch_generic_connection_error(self):
-        with pytest.raises(errors.ConnectionErrorGeneric):
-            with mock.patch(
-                'compose.cli.command.call_silently',
-                autospec=True,
-                side_effect=[0, 1]
-            ):
-                with friendly_error_message():
-                    raise ConnectionError()
-
-
 class TestGetConfigPathFromOptions(object):
 
     def test_path_from_options(self):

tests/unit/cli/errors_test.py

@@ -0,0 +1,51 @@
+from __future__ import absolute_import
+from __future__ import unicode_literals
+
+import pytest
+from docker.errors import APIError
+from requests.exceptions import ConnectionError
+
+from compose.cli import errors
+from compose.cli.errors import handle_connection_errors
+from tests import mock
+
+
+@pytest.yield_fixture
+def mock_logging():
+    with mock.patch('compose.cli.errors.log', autospec=True) as mock_log:
+        yield mock_log
+
+
+def patch_call_silently(side_effect):
+    return mock.patch(
+        'compose.cli.errors.call_silently',
+        autospec=True,
+        side_effect=side_effect)
+
+
+class TestHandleConnectionErrors(object):
+
+    def test_generic_connection_error(self, mock_logging):
+        with pytest.raises(errors.ConnectionError):
+            with patch_call_silently([0, 1]):
+                with handle_connection_errors(mock.Mock()):
+                    raise ConnectionError()
+
+        _, args, _ = mock_logging.error.mock_calls[0]
+        assert "Couldn't connect to Docker daemon at" in args[0]
+
+    def test_api_error_version_mismatch(self, mock_logging):
+        with pytest.raises(errors.ConnectionError):
+            with handle_connection_errors(mock.Mock(api_version='1.22')):
+                raise APIError(None, None, "client is newer than server")
+
+        _, args, _ = mock_logging.error.mock_calls[0]
+        assert "Docker Engine of version 1.10.0 or greater" in args[0]
+
+    def test_api_error_version_other(self, mock_logging):
+        msg = "Something broke!"
+        with pytest.raises(errors.ConnectionError):
+            with handle_connection_errors(mock.Mock(api_version='1.22')):
+                raise APIError(None, None, msg)
+
+        mock_logging.error.assert_called_once_with(msg)