Skip to content

[16.0] rest_log: Ensure retrying mechanism is working properly #520

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 14, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions base_rest_demo/services/exception_services.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# Copyright 2018 ACSONE SA/NV
# License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl).

from psycopg2 import errorcodes
from psycopg2.errors import OperationalError
from werkzeug.exceptions import MethodNotAllowed

from odoo import _
@@ -12,9 +14,13 @@
ValidationError,
)
from odoo.http import SessionExpiredException
from odoo.service.model import MAX_TRIES_ON_CONCURRENCY_FAILURE

from odoo.addons.base_rest.components.service import to_int
from odoo.addons.component.core import Component

_CPT_RETRY = 0


class ExceptionService(Component):
_inherit = "base.rest.service"
@@ -90,6 +96,23 @@ def bare_exception(self):
"""
raise IOError("My IO error")

def retryable_error(self, nbr_retries):
"""This method is used in the test suite to check that the retrying
functionality in case of concurrency error on the database is working
correctly for retryable exceptions.

The output will be the number of retries that have been done.

This method is mainly used to test the retrying functionality
"""
global _CPT_RETRY
if _CPT_RETRY < nbr_retries:
_CPT_RETRY += 1
raise FakeConcurrentUpdateError("fake error")
tryno = _CPT_RETRY
_CPT_RETRY = 0
return {"retries": tryno}

# Validator
def _validator_user_error(self):
return {}
@@ -138,3 +161,22 @@ def _validator_bare_exception(self):

def _validator_return_bare_exception(self):
return {}

def _validator_retryable_error(self):
return {
"nbr_retries": {
"type": "integer",
"required": True,
"default": MAX_TRIES_ON_CONCURRENCY_FAILURE,
"coerce": to_int,
}
}

def _validator_return_retryable_error(self):
return {"retries": {"type": "integer"}}


class FakeConcurrentUpdateError(OperationalError):
@property
def pgcode(self):
return errorcodes.SERIALIZATION_FAILURE
14 changes: 14 additions & 0 deletions base_rest_demo/tests/test_exception.py
Original file line number Diff line number Diff line change
@@ -104,3 +104,17 @@ def test_bare_exception(self):
self.assertEqual(response.headers["content-type"], "application/json")
body = json.loads(response.content.decode("utf-8"))
self.assertDictEqual(body, {"code": 500, "name": "Internal Server Error"})

@odoo.tools.mute_logger("odoo.addons.base_rest.http", "odoo.http")
def test_retrying(self):
"""Test that the retrying mechanism is working as expected with the
FastAPI endpoints in case of POST request with a file.
"""
nbr_retries = 3
response = self.url_open(
"%s/retryable_error" % self.url,
'{"nbr_retries": %d}' % nbr_retries,
timeout=20000,
)
self.assertEqual(response.status_code, 200, response.content)
self.assertDictEqual(response.json(), {"retries": nbr_retries})
11 changes: 11 additions & 0 deletions rest_log/components/service.py
Original file line number Diff line number Diff line change
@@ -7,10 +7,12 @@
import logging
import traceback

from psycopg2.errors import OperationalError
from werkzeug.urls import url_encode, url_join

from odoo import exceptions, registry
from odoo.http import Response, request
from odoo.service.model import PG_CONCURRENCY_ERRORS_TO_RETRY

from odoo.addons.base_rest.http import JSONEncoder
from odoo.addons.component.core import AbstractComponent
@@ -111,6 +113,15 @@ def _dispatch_exception(
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
):
raise orig_exception
raise exception_klass(exc_msg, log_entry_url) from orig_exception

def _get_exception_message(self, exception):
10 changes: 10 additions & 0 deletions rest_log/tests/common.py
Original file line number Diff line number Diff line change
@@ -4,6 +4,9 @@

import contextlib

from psycopg2 import errorcodes
from psycopg2.errors import OperationalError

from odoo import exceptions

from odoo.addons.base_rest import restapi
@@ -42,6 +45,7 @@ def fail(self, how):
"value": ValueError,
"validation": exceptions.ValidationError,
"user": exceptions.UserError,
"retryable": FakeConcurrentUpdateError,
}
raise exc[how]("Failed as you wanted!")

@@ -61,3 +65,9 @@ def _get_mocked_request(self, env=None, httprequest=None, extra_headers=None):
headers.update(extra_headers or {})
mocked_request.httprequest.headers = headers
yield mocked_request


class FakeConcurrentUpdateError(OperationalError):
@property
def pgcode(self):
return errorcodes.SERIALIZATION_FAILURE
65 changes: 64 additions & 1 deletion rest_log/tests/test_db_logging.py
Original file line number Diff line number Diff line change
@@ -13,7 +13,7 @@
from odoo.addons.component.tests.common import new_rollbacked_env
from odoo.addons.rest_log import exceptions as log_exceptions # pylint: disable=W7950

from .common import TestDBLoggingMixin
from .common import FakeConcurrentUpdateError, TestDBLoggingMixin


class TestDBLogging(TransactionRestServiceRegistryCase, TestDBLoggingMixin):
@@ -374,3 +374,66 @@ def test_log_exception_value(self):
self._test_exception(
"value", log_exceptions.RESTServiceDispatchException, "ValueError", "severe"
)


class TestDBLoggingRetryableError(
TransactionRestServiceRegistryCase, TestDBLoggingMixin
):
@classmethod
def setUpClass(cls):
super().setUpClass()
cls._setup_registry(cls)

@classmethod
def tearDownClass(cls):
# pylint: disable=W8110
cls._teardown_registry(cls)
super().tearDownClass()

def _test_exception(self, test_type, wrapping_exc, exc_name, severity):
log_model = self.env["rest.log"].sudo()
initial_entries = log_model.search([])
# Context: we are running in a transaction case which uses savepoints.
# The log machinery is going to rollback the transation when catching errors.
# Hence we need a completely separated env for the service.
with new_rollbacked_env() as new_env:
# 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):
try:
service.dispatch("fail", test_type)
except Exception as err:
# Not using `assertRaises` to inspect the exception directly
self.assertTrue(isinstance(err, wrapping_exc))
self.assertEqual(
service._get_exception_message(err), "Failed as you wanted!"
)

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])

@staticmethod
def _get_test_controller(class_or_instance, root_path=None):
return super()._get_test_controller(
class_or_instance, root_path="/test_log_exception_retryable/"
)

def test_log_exception_retryable(self):
# retryable error must bubble up to the retrying mechanism
self._test_exception(
"retryable",
FakeConcurrentUpdateError,
"odoo.addons.rest_log.tests.common.FakeConcurrentUpdateError",
"warning",
)