Skip to content

Commit

Permalink
Improve error handling for TCP connections
Browse files Browse the repository at this point in the history
In the abstract DNSService's _dns_handle_tcp method, error handling
is broken in a way that stops the main loop for handling TCP
connections.

Because socket.timeout is a subclass of socket.error, the error
handling block for socket.timeout is never reached.

Because of this, error handling of a TCP timeout is sent to the
socket.error block.  Due to the way eventlet hijacks these errors,
the errorcode is not available and a KeyError is raised.  This
KeyError interferes with the main loop because it is not caught.

Further improvement may include ensuring that these main loops
can never die due to unexpected exceptions.

Many thanks to Erik Andersson for pointing out the issue, which
was seemingly innocuous but ended up being the cause of our
problems.

Closes-bug: 1549980
Change-Id: I47e1260a0818cc42cbd56e4d296e083f8fcbbae5
  • Loading branch information
Rahman Syed authored and Kiall Mac Innes committed Mar 1, 2016
1 parent 648577d commit a42f0ba
Showing 1 changed file with 8 additions and 5 deletions.
13 changes: 8 additions & 5 deletions designate/service.py
Expand Up @@ -264,17 +264,20 @@ def _dns_handle_tcp(self):
break
payload += data

# NOTE: Any uncaught exceptions will result in the main loop
# ending unexpectedly. Ensure proper ordering of blocks, and
# ensure no exceptions are generated from within.
except socket.timeout:
client.close()
LOG.warn(_LW("TCP Timeout from: %(host)s:%(port)d") %
{'host': addr[0], 'port': addr[1]})

except socket.error as e:
client.close()
errname = errno.errorcode[e.args[0]]
LOG.warn(_LW("Socket error %(err)s from: %(host)s:%(port)d") %
{'host': addr[0], 'port': addr[1], 'err': errname})

except socket.timeout:
client.close()
LOG.warn(_LW("TCP Timeout from: %(host)s:%(port)d") %
{'host': addr[0], 'port': addr[1]})

except struct.error:
client.close()
LOG.warn(_LW("Invalid packet from: %(host)s:%(port)d") %
Expand Down

0 comments on commit a42f0ba

Please sign in to comment.