Improve error handling for nailgun client connection attempts

Review Request #2869 - Created Sept. 23, 2015 and submitted

Information
Kris Wilson
pants
twitter:kwlzn/ng_client_fixes
2238, 2246
5f804b2...
Reviewers
pants-reviews
benjyw, jsirois, mateor, nhoward_tw, stuhood

Fixes #2238

  • Use 127.0.0.1 vs localhost for nailgun default host. this prevents a situation where a broken /etc/hosts can affect pants ability to connect to the nailgun (currently raises an unhandled and generic socket.gaierror).
  • Add a specialized NailgunClient.NailgunConnectionError exception type.
  • Convert NailgunClient.try_connect() from socket.connect_ex() to socket.connect() with added catching of socket.gaierror which is not covered by socket.connect_ex().
  • Make NailgunClient.try_connect() raise a helpful NailgunConnectionError exception on failure vs returning None.
  • Propagate the helpful NailgunConnectionError in both NailgunClient.call() and NailgunExecutor.ensure_connectable().
  • Improve debug logging.
https://travis-ci.org/pantsbuild/pants/builds/81720419 + manual testing.


Before/After for a broken /etc/hosts:

    Exception message: [Errno 8] nodename nor servname provided, or not known
vs
    Exception message: Problem connecting to nailgun server at localhost:55261: gaierror(8, 'nodename nor servname provided, or not known')


Before (with resident ng)/Before (without resident ng)/After for an unconnectable nailgun:

    Exception message: Failed to connect to ng server.
vs
    Exception message: 'NoneType' object has no attribute 'close'
vs
    Exception message: Problem connecting to nailgun server at 127.0.0.1:9999: error(61, 'Connection refused')
Mateo Rodriguez
John Sirois
Kris Wilson
John Sirois
Nick Howard (Twitter)
Kris Wilson
Review request changed

Status: Closed (submitted)

Change Summary:

thanks gents! submitted @ 35ba6bfdb63cfedbedfd28c1705996e582178064

Loading...