[16.0][IMP] rest_log: Don't log retried error

rest_log/components/service.py

@@ -3,6 +3,7 @@
 # @author Simone Orsi <simahawk@gmail.com>
 # License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html).
 
+import inspect
 import json
 import logging
 import traceback
@@ -93,9 +94,37 @@
         except Exception as e:
             _logger.exception("Rest Log Error Creation: %s", e)
 
+    def _is_request_will_be_retried(self):
+        # we need to analyze the current call stack to determine if
+        # a 'tryleft' local variable is set, which indicates that
+        # the current request is being retried.
+        # This is a workaround for the fact that we cannot access
+        # the 'tryleft' variable from the request context, as it is
+        # not part of the request object.
+        # This is a hack, but it is the only way to determine if
+        # the current request is being retried.
+        stack = inspect.stack()
+        for frame in stack:
+            if "tryleft" in frame.frame.f_locals:
+                return frame.frame.f_locals["tryleft"]
+        return False
+
     def _dispatch_exception(
         self, method_name, exception_klass, orig_exception, *args, params=None
     ):
+        must_return_original_exception = False
+        if (
+            isinstance(orig_exception, OperationalError)
+            and orig_exception.pgcode in PG_CONCURRENCY_ERRORS_TO_RETRY
+        ):
+            must_return_original_exception = True
+            if self._is_request_will_be_retried():
+                # If we are in a retry, we don't log the error in the database,
+                # as it will be retried and logged again. And we let the
+                # OperationalError bubble up to the retrying mechanism where
+                # it will be handled by the default handler at the end of the chain.
+                raise orig_exception
+
         exc_msg, log_entry_url = None, None  # in case it fails below
         try:
             exc_msg = self._get_exception_message(orig_exception)
@@ -113,14 +142,7 @@
                 log_entry_url = self._get_log_entry_url(log_entry)
         except Exception as e:
             _logger.exception("Rest Log Error Creation: %s", e)
-        # let the OperationalError bubble up to the retrying mechanism
-        # We can't wrap the OperationalError because we want to let it
-        # bubble up to the retrying mechanism, it will be handled by
-        # the default handler at the end of the chain.
-        if (
-            isinstance(orig_exception, OperationalError)
-            and orig_exception.pgcode in PG_CONCURRENCY_ERRORS_TO_RETRY
-        ):
+        if must_return_original_exception:
             raise orig_exception
         raise exception_klass(exc_msg, log_entry_url) from orig_exception
 

rest_log/tests/test_db_logging.py

@@ -390,7 +390,9 @@ def tearDownClass(cls):
         cls._teardown_registry(cls)
         super().tearDownClass()
 
-    def _test_exception(self, test_type, wrapping_exc, exc_name, severity):
+    def _test_exception(
+        self, test_type, wrapping_exc, exc_name, severity, will_be_retried=True
+    ):
         log_model = self.env["rest.log"].sudo()
         initial_entries = log_model.search([])
         # Context: we are running in a transaction case which uses savepoints.
@@ -400,7 +402,9 @@ def _test_exception(self, test_type, wrapping_exc, exc_name, severity):
             # Init fake collection w/ new env
             collection = _PseudoCollection(self._collection_name, new_env)
             service = self._get_service(self, collection=collection)
-            with self._get_mocked_request(env=new_env):
+            with self._get_mocked_request(env=new_env), mock.patch.object(
+                service, "_is_request_will_be_retried", return_value=will_be_retried
+            ):
                 try:
                     service.dispatch("fail", test_type)
                 except Exception as err:
@@ -413,15 +417,20 @@ def _test_exception(self, test_type, wrapping_exc, exc_name, severity):
         with new_rollbacked_env() as new_env:
             log_model = new_env["rest.log"].sudo()
             entry = log_model.search([]) - initial_entries
-            expected = {
-                "collection": service._collection,
-                "state": "failed",
-                "result": "null",
-                "exception_name": exc_name,
-                "exception_message": "Failed as you wanted!",
-                "severity": severity,
-            }
-            self.assertRecordValues(entry, [expected])
+            if will_be_retried:
+                # retryable error must bubble up to the retrying mechanism
+                self.assertEqual(len(entry), 0)
+            else:
+                # not retryable, so we log it
+                expected = {
+                    "collection": service._collection,
+                    "state": "failed",
+                    "result": "null",
+                    "exception_name": exc_name,
+                    "exception_message": "Failed as you wanted!",
+                    "severity": severity,
+                }
+                self.assertRecordValues(entry, [expected])
 
     @staticmethod
     def _get_test_controller(class_or_instance, root_path=None):
@@ -436,4 +445,12 @@ def test_log_exception_retryable(self):
             FakeConcurrentUpdateError,
             "odoo.addons.rest_log.tests.common.FakeConcurrentUpdateError",
             "warning",
+            will_be_retried=True,
+        )
+        self._test_exception(
+            "retryable",
+            FakeConcurrentUpdateError,
+            "odoo.addons.rest_log.tests.common.FakeConcurrentUpdateError",
+            "warning",
+            will_be_retried=False,
         )