diff --git a/src/python/pants/java/nailgun_client.py b/src/python/pants/java/nailgun_client.py index 75edb9502a2..3330a84ca80 100644 --- a/src/python/pants/java/nailgun_client.py +++ b/src/python/pants/java/nailgun_client.py @@ -5,6 +5,7 @@ from __future__ import (absolute_import, division, generators, nested_scopes, print_function, unicode_literals, with_statement) +import logging import os import select import socket @@ -14,6 +15,9 @@ from functools import partial +logger = logging.getLogger(__name__) + + class NailgunSession(object): """Handles a single nailgun command session.""" @@ -148,9 +152,12 @@ class NailgunClient(object): """ class NailgunError(Exception): - """Indicates an error connecting to or interacting with a nailgun server.""" + """Indicates an error interacting with a nailgun server.""" - DEFAULT_NG_HOST = 'localhost' + class NailgunConnectionError(NailgunError): + """Indicates an error upon initial connect to the nailgun server.""" + + DEFAULT_NG_HOST = '127.0.0.1' DEFAULT_NG_PORT = 2113 # For backwards compatibility with nails expecting the ng c client special env vars. @@ -168,7 +175,7 @@ def __init__(self, workdir=None): """Creates a nailgun client that can be used to issue zero or more nailgun commands. - :param string host: the nailgun server to contact (defaults to localhost) + :param string host: the nailgun server to contact (defaults to '127.0.0.1') :param int port: the port the nailgun server is listening on (defaults to the default nailgun port: 2113) :param file ins: a file to read command standard input from (defaults to stdin) - can be None @@ -187,8 +194,21 @@ def __init__(self, self.execute = self.__call__ def try_connect(self): + """Creates a socket, connects it to the nailgun and returns the connected socket. + + :returns: a connected `socket.socket`. + :raises: `NailgunClient.NailgunConnectionError` on failure to connect. + """ sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) - return sock if sock.connect_ex((self._host, self._port)) == 0 else None + try: + sock.connect((self._host, self._port)) + except (socket.error, socket.gaierror) as e: + logger.debug('Encountered socket exception {!r} when attempting connect to nailgun'.format(e)) + sock.close() + raise self.NailgunConnectionError( + 'Problem connecting to nailgun server at {}:{}: {!r}'.format(self._host, self._port, e)) + else: + return sock def __call__(self, main_class, cwd=None, *args, **environment): """Executes the given main_class with any supplied args in the given environment. @@ -203,22 +223,23 @@ def __call__(self, main_class, cwd=None, *args, **environment): """ environment = dict(self.ENV_DEFAULTS.items() + environment.items()) cwd = cwd or self._workdir + + # N.B. This can throw NailgunConnectionError. sock = self.try_connect() - if not sock: - raise self.NailgunError('Problem connecting to nailgun server' - ' {}:{}'.format(self._host, self._port)) session = NailgunSession(sock, self._ins, self._out, self._err) try: return session.execute(cwd, main_class, *args, **environment) except socket.error as e: - raise self.NailgunError('Problem contacting nailgun server {}:{}:' - ' {}'.format(self._host, self._port, e)) + raise self.NailgunError('Problem communicating with nailgun server at {}:{}: {!r}' + .format(self._host, self._port, e)) except session.ProtocolError as e: - raise self.NailgunError('Problem executing the nailgun protocol with nailgun server {}:{}:' - ' {}'.format(self._host, self._port, e)) + raise self.NailgunError('Problem in nailgun protocol with nailgun server at {}:{}: {!r}' + .format(self._host, self._port, e)) finally: sock.close() def __repr__(self): - return 'NailgunClient(host={!r}, port={!r}, workdir={!r})'.format(self._host, self._port, self._workdir) + return 'NailgunClient(host={!r}, port={!r}, workdir={!r})'.format(self._host, + self._port, + self._workdir) diff --git a/src/python/pants/java/nailgun_executor.py b/src/python/pants/java/nailgun_executor.py index ec795a88fbb..8bb11a7ee26 100644 --- a/src/python/pants/java/nailgun_executor.py +++ b/src/python/pants/java/nailgun_executor.py @@ -12,6 +12,7 @@ import select import threading import time +from contextlib import closing from six import string_types from twitter.common.collections import maybe_list @@ -210,20 +211,16 @@ def _create_ngclient(self, port, stdout, stderr): def ensure_connectable(self, nailgun): """Ensures that a nailgun client is connectable or raises NailgunError.""" - attempt_count = 0 + attempt_count = 1 while 1: - if attempt_count > self._connect_attempts: - logger.debug('Failed to connect to ng after {count} attempts' - .format(count=self._connect_attempts)) - raise NailgunClient.NailgunError('Failed to connect to ng server.') - try: - sock = nailgun.try_connect() - if sock: - logger.debug('Connected to ng server {server!r}'.format(server=self)) + with closing(nailgun.try_connect()) as sock: + logger.debug('Verified new ng server is connectable at {}'.format(sock.getpeername())) return - finally: - sock.close() + except nailgun.NailgunConnectionError: + if attempt_count >= self._connect_attempts: + logger.debug('Failed to connect to ng after {} attempts'.format(self._connect_attempts)) + raise # Re-raise the NailgunConnectionError which provides more context to the user. attempt_count += 1 time.sleep(self.WAIT_INTERVAL_SEC)