From 0a091055d24bec9501e918353fe1c5009f88dc0c Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 8 Mar 2016 15:39:11 -0500 Subject: [PATCH] Improve handling of connection errors and error messages. Signed-off-by: Daniel Nephin --- compose/cli/command.py | 35 ++------- compose/cli/errors.py | 125 +++++++++++++++++++++++++-------- compose/cli/main.py | 39 ++-------- tests/unit/cli/command_test.py | 16 ----- tests/unit/cli/errors_test.py | 51 ++++++++++++++ 5 files changed, 153 insertions(+), 113 deletions(-) create mode 100644 tests/unit/cli/errors_test.py diff --git a/compose/cli/command.py b/compose/cli/command.py index 98de21044..55f6df01a 100644 --- a/compose/cli/command.py +++ b/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 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): +def project_from_options(project_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): - config_details = config.find(base_dir, config_path) +def get_project(project_dir, config_path=None, project_name=None, verbose=False): + 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) diff --git a/compose/cli/errors.py b/compose/cli/errors.py index 03d6a50c6..a16cad2f5 100644 --- a/compose/cli/errors.py +++ b/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): - def __init__(self): - super(DockerNotFoundMac, self).__init__(""" - Couldn't connect to Docker daemon. You might need to install docker-osx: - - https://github.com/noplay/docker-osx - """) +class ConnectionError(Exception): + pass -class DockerNotFoundUbuntu(UserError): - def __init__(self): - super(DockerNotFoundUbuntu, self).__init__(""" - Couldn't connect to Docker daemon. You might need to install Docker: - - https://docs.docker.com/engine/installation/ubuntulinux/ - """) +@contextlib.contextmanager +def handle_connection_errors(client): + try: + yield + except SSLError as e: + 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 __init__(self): - super(DockerNotFoundGeneric, self).__init__(""" - Couldn't connect to Docker daemon. You might need to install Docker: +def log_api_error(e, client_version): + if 'client is newer than server' not in e.explanation: + log.error(e.explanation) + 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 __init__(self): - super(ConnectionErrorDockerMachine, self).__init__(""" - Couldn't connect to Docker daemon - you might need to run `docker-machine start default`. - """) +def exit_with_error(msg): + log.error(dedent(msg).strip()) + raise ConnectionError() -class ConnectionErrorGeneric(UserError): - def __init__(self, url): - super(ConnectionErrorGeneric, self).__init__(""" - Couldn't connect to Docker daemon at %s - is it running? +docker_not_found_mac = """ + Couldn't connect to Docker daemon. You might need to install docker-osx: - If it's at a non-standard location, specify the URL with the DOCKER_HOST environment variable. - """ % url) + https://github.com/noplay/docker-osx +""" + + +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. +""" diff --git a/compose/cli/main.py b/compose/cli/main.py index b020a14b7..663621682 100644 --- a/compose/cli/main.py +++ b/compose/cli/main.py @@ -11,18 +11,14 @@ import sys from inspect import getdoc from operator import attrgetter -from docker.errors import APIError -from requests.exceptions import ReadTimeout - +from . import errors 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: - 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 - ) + except errors.ConnectionError: 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) diff --git a/tests/unit/cli/command_test.py b/tests/unit/cli/command_test.py index 1ca671fed..b524a5f3b 100644 --- a/tests/unit/cli/command_test.py +++ b/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): diff --git a/tests/unit/cli/errors_test.py b/tests/unit/cli/errors_test.py new file mode 100644 index 000000000..a99356b44 --- /dev/null +++ b/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)