From 3cc24fe876e47115cbe2993216d6e80a585f4a8e Mon Sep 17 00:00:00 2001 From: Philip Jenvey Date: Tue, 23 Aug 2016 11:17:47 -0700 Subject: [PATCH] fix: handle more errors to connection nodes * prefer base classes ConnectError/ConnectionClosed/ResponseFailed, covering ConnectionRefused/UserError, ConnectionLost/Done, ResponseNeverReceived * handle ResponseFailed (ResponseNeverReceived) in the routers as a broken connection * trap these in PushServerProtocol._notify_node closes #613, #554 --- autopush/router/simple.py | 7 ++++--- autopush/websocket.py | 17 ++++++++--------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/autopush/router/simple.py b/autopush/router/simple.py index 4f55b2d5..0fe13680 100644 --- a/autopush/router/simple.py +++ b/autopush/router/simple.py @@ -23,10 +23,11 @@ ) from twisted.internet.error import ( ConnectError, + ConnectionClosed, ConnectionRefusedError, - UserError ) from twisted.logger import Logger +from twisted.web._newclient import ResponseFailed from twisted.web.client import FileBodyProducer from autopush.protocol import IgnoreBody @@ -100,7 +101,7 @@ def route_notification(self, notification, uaid_data): try: result = yield self._send_notification(uaid, node_id, notification) - except (ConnectError, UserError, ConnectionRefusedError) as exc: + except (ConnectError, ConnectionClosed, ResponseFailed) as exc: self.metrics.increment("updates.client.host_gone") dead_cache.put(node_key(node_id), True) yield deferToThread(router.clear_node, @@ -157,7 +158,7 @@ def route_notification(self, notification, uaid_data): returnValue(self.stored_response(notification)) try: result = yield self._send_notification_check(uaid, node_id) - except (ConnectError, UserError, ConnectionRefusedError) as exc: + except (ConnectError, ConnectionClosed, ResponseFailed) as exc: self.metrics.increment("updates.client.host_gone") dead_cache.put(node_key(node_id), True) if isinstance(exc, ConnectionRefusedError): diff --git a/autopush/websocket.py b/autopush/websocket.py index 88d5b098..7c53f380 100644 --- a/autopush/websocket.py +++ b/autopush/websocket.py @@ -48,16 +48,16 @@ DeferredList, CancelledError ) -from twisted.web._newclient import ResponseNeverReceived from twisted.internet.error import ( - ConnectError, ConnectionRefusedError, UserError, - ConnectionLost, ConnectionDone + ConnectError, + ConnectionClosed ) from twisted.internet.interfaces import IProducer from twisted.internet.threads import deferToThread from twisted.logger import Logger from twisted.protocols import policies from twisted.python import failure +from twisted.web._newclient import ResponseFailed from twisted.web.resource import Resource from zope.interface import implements @@ -293,6 +293,9 @@ def f(): def trap_cancel(self, fail): fail.trap(CancelledError) + def trap_connection_err(self, fail): + fail.trap(ConnectError, ConnectionClosed, ResponseFailed) + def force_retry(self, func, *args, **kwargs): """Forcefully retry a function in a thread until it doesn't error @@ -568,6 +571,7 @@ def _notify_node(self, result): "PUT", url.encode("utf8"), ).addCallback(IgnoreBody.ignore) + d.addErrback(self.trap_connection_err) d.addErrback(self.log_failure, extra="Failed to notify node") def returnError(self, messageType, reason, statusCode, close=True, @@ -776,12 +780,7 @@ def _check_other_nodes(self, result, url=DEFAULT_WS_ERR): "DELETE", url.encode("utf8"), ) - d.addErrback(lambda f: f.trap(ConnectError, - ConnectionRefusedError, - UserError, - ResponseNeverReceived, - ConnectionLost, - ConnectionDone)) + d.addErrback(self.trap_connection_err) d.addErrback(self.log_failure, extra="Failed to delete old node")