Ignore 404s on close/destroy requests due to possible retry

Author: bjester

morango/sync/operations.py

@@ -1390,9 +1390,8 @@ def close_transfer_session(self, context):
         Closes remote transfer session
 
         :type context: NetworkSessionContext
-        :return: The Response
         """
-        return context.connection._close_transfer_session(context.transfer_session)
+        context.connection._close_transfer_session(context.transfer_session)
 
     def put_buffers(self, context, buffers):
         """
@@ -1763,8 +1762,5 @@ def handle(self, context):
         """
         :type context: NetworkSessionContext
         """
-        response = self.close_transfer_session(context)
-        remote_status = transfer_statuses.COMPLETED
-        if response.status_code < 200 or response.status_code >= 300:
-            remote_status = transfer_statuses.ERRORED
-        return remote_status
+        self.close_transfer_session(context)
+        return transfer_statuses.COMPLETED

morango/sync/syncsession.py

@@ -6,6 +6,7 @@
 import os
 import socket
 import uuid
+from functools import wraps
 from io import BytesIO
 from urllib.parse import urljoin
 from urllib.parse import urlparse
@@ -53,6 +54,7 @@
 
 DBBackend = load_backend(connection)
 
+
 def _join_with_logical_operator(lst, operator):
     op = ") {operator} (".format(operator=operator)
     return "(({items}))".format(items=op.join(lst))
@@ -77,6 +79,25 @@ def _get_client_ip_for_server(server_host, server_port):
     return IP
 
 
+def ignore_404(target):
+    """
+    Decorator that wraps callables to ignore 404s caused by its use of requests
+    :param target: A callable
+    :return: A callable
+    """
+    @wraps(target)
+    def wrapper(*args, **kwargs):
+        try:
+            return target(*args, **kwargs)
+        except HTTPError as e:
+            if e.response is None or e.response.status_code != 404:
+                raise e
+            else:
+                logger.debug(f"Ignoring 404 raised by {target.__name__}")
+
+    return wrapper
+
+
 # borrowed from https://github.com/django/django/blob/1.11.20/django/utils/text.py#L295
 def compress_string(s, compresslevel=9):
     zbuf = BytesIO()
@@ -479,13 +500,15 @@ def _update_transfer_session(self, data, transfer_session):
             json=data,
         )
 
+    @ignore_404
     def _close_transfer_session(self, transfer_session):
-        return self.session.delete(
+        self.session.delete(
             self.urlresolve(api_urls.TRANSFERSESSION, lookup=transfer_session.id)
         )
 
+    @ignore_404
     def _close_sync_session(self, sync_session):
-        return self.session.delete(
+        self.session.delete(
             self.urlresolve(api_urls.SYNCSESSION, lookup=sync_session.id)
         )
 

tests/testapp/tests/sync/test_syncsession.py

@@ -21,6 +21,7 @@
 from morango.models.certificates import Key
 from morango.models.certificates import ScopeDefinition
 from morango.models.core import SyncSession
+from morango.models.core import TransferSession
 from morango.models.fields.crypto import SharedKey
 from morango.sync.context import LocalSessionContext
 from morango.sync.context import NetworkSessionContext
@@ -236,6 +237,66 @@ def create(**data):
         self.network_connection.close_sync_session(client.sync_session)
         self.assertEqual(SyncSession.objects.filter(active=True).count(), 0)
 
+    def test_close_transfer_session_ignores_404(self):
+        mock_response = mock.Mock()
+        mock_response.status_code = 404
+        transfer_session = mock.Mock(spec=TransferSession)
+        transfer_session.id = uuid.uuid4().hex
+
+        with mock.patch.object(
+            self.network_connection.session,
+            "delete",
+            side_effect=HTTPError(response=mock_response),
+        ):
+            # should not raise even though the server returned 404
+            self.network_connection._close_transfer_session(transfer_session)
+
+    def test_close_sync_session_ignores_404(self):
+        mock_response = mock.Mock()
+        mock_response.status_code = 404
+        sync_session = mock.Mock(spec=SyncSession)
+        sync_session.id = uuid.uuid4().hex
+
+        with mock.patch.object(
+            self.network_connection.session,
+            "delete",
+            side_effect=HTTPError(response=mock_response),
+        ):
+            # should not raise even though the server returned 404
+            self.network_connection._close_sync_session(sync_session)
+
+    def test_close_sync_session_raises_500(self):
+        mock_response = mock.Mock()
+        mock_response.status_code = 500
+        sync_session = mock.Mock(spec=SyncSession)
+        sync_session.id = uuid.uuid4().hex
+
+        e = HTTPError(response=mock_response)
+
+        with mock.patch.object(
+            self.network_connection.session,
+            "delete",
+            side_effect=e,
+        ):
+            with self.assertRaises(type(e)) as raised:
+                self.network_connection._close_sync_session(sync_session)
+            self.assertEqual(e, raised.exception)
+
+    def test_close_sync_session_raises_no_response(self):
+        sync_session = mock.Mock(spec=SyncSession)
+        sync_session.id = uuid.uuid4().hex
+
+        e = HTTPError()
+
+        with mock.patch.object(
+            self.network_connection.session,
+            "delete",
+            side_effect=e,
+        ):
+            with self.assertRaises(type(e)) as raised:
+                self.network_connection._close_sync_session(sync_session)
+            self.assertEqual(e, raised.exception)
+
     @mock.patch.object(SyncSession.objects, "create")
     def test_resume_sync_session(self, mock_create):
         def create(**data):