diff --git a/spp_attachment_av_scan/README.rst b/spp_attachment_av_scan/README.rst
index 5e8c7fc23..438880b16 100644
--- a/spp_attachment_av_scan/README.rst
+++ b/spp_attachment_av_scan/README.rst
@@ -134,6 +134,15 @@ External: ``pyclamd`` (Python library for ClamAV integration)
Changelog
=========
+19.0.2.0.1
+~~~~~~~~~~
+
+- fix: re-raise database errors (``psycopg2.Error``,
+ ``ConcurrencyError``) from the create/write scan-queue hooks instead
+ of swallowing them, so transient serialization failures reach Odoo's
+ transaction retry machinery instead of poisoning the transaction for
+ unrelated downstream code
+
19.0.2.0.0
~~~~~~~~~~
diff --git a/spp_attachment_av_scan/__manifest__.py b/spp_attachment_av_scan/__manifest__.py
index 0d93ad2db..2af626f15 100644
--- a/spp_attachment_av_scan/__manifest__.py
+++ b/spp_attachment_av_scan/__manifest__.py
@@ -1,7 +1,7 @@
{ # pylint: disable=pointless-statement
"name": "OpenSPP Attachment Antivirus Scan",
"category": "OpenSPP",
- "version": "19.0.2.0.0",
+ "version": "19.0.2.0.1",
"sequence": 1,
"author": "OpenSPP.org",
"website": "https://github.com/OpenSPP/OpenSPP2",
diff --git a/spp_attachment_av_scan/models/ir_attachment.py b/spp_attachment_av_scan/models/ir_attachment.py
index 5442e2dad..0d4a5265b 100644
--- a/spp_attachment_av_scan/models/ir_attachment.py
+++ b/spp_attachment_av_scan/models/ir_attachment.py
@@ -3,11 +3,29 @@
import json
import logging
+import psycopg2
+
from odoo import Command, _, api, fields, models
-from odoo.exceptions import AccessError, UserError
+from odoo.exceptions import AccessError, ConcurrencyError, UserError
_logger = logging.getLogger(__name__)
+#: Queueing a malware scan is best-effort — a scan that cannot be enqueued must not
+#: block the attachment write. A **database** error is categorically different: it
+#: leaves the transaction unusable, so swallowing one converts a recoverable fault
+#: into an unrelated failure somewhere downstream.
+#:
+#: This is a superset of the classes ``odoo.service.model.retrying`` recovers from
+#: by rolling back and re-running the request (``IntegrityError``, ``OperationalError``,
+#: ``ConcurrencyError`` — see ``odoo/service/model.py``); the non-retryable rest of
+#: ``psycopg2.Error`` then surfaces with the true traceback instead of poisoning
+#: downstream code. ``SerializationFailure``
+#: resolves through that tuple via
+#: ``SerializationFailure -> TransactionRollbackError -> OperationalError``, so a
+#: concurrent-update conflict on an attachment is retried transparently — *unless*
+#: something catches it first.
+_MUST_NOT_SWALLOW = (psycopg2.Error, ConcurrencyError)
+
QUARANTINE_PROVIDER_PARAM = "spp_attachment_av_scan.quarantine_encryption_provider_id"
QUARANTINE_RETENTION_DAYS_PARAM = "spp_attachment_av_scan.quarantine_retention_days"
DEFAULT_QUARANTINE_RETENTION_DAYS = 90
@@ -92,6 +110,11 @@ def create(self, vals_list):
priority=20,
)._scan_for_malware()
_logger.info("Queued malware scan for attachment ID %s", attachment.id)
+ except _MUST_NOT_SWALLOW:
+ # Never swallow: see ``_MUST_NOT_SWALLOW``. Re-raise so the
+ # request is rolled back and retried instead of continuing on a
+ # dead transaction.
+ raise
except Exception as error:
_logger.error(
"Failed to queue malware scan for attachment ID %s: %s",
@@ -130,6 +153,14 @@ def write(self, vals):
"Queued malware scan for updated attachment ID %s",
attachment.id,
)
+ except _MUST_NOT_SWALLOW:
+ # Never swallow: see ``_MUST_NOT_SWALLOW``. This is the exact
+ # site that turned a transient "could not serialize access due
+ # to concurrent update" on an attachment into an
+ # InFailedSqlTransaction reported from an unrelated menu-icon
+ # lookup, on every module upgrade, with the real cause visible
+ # only as a stray ERROR line in the server log.
+ raise
except Exception as error:
_logger.error(
"Failed to queue malware scan for updated attachment ID %s: %s",
diff --git a/spp_attachment_av_scan/readme/HISTORY.md b/spp_attachment_av_scan/readme/HISTORY.md
index 4aaf9afef..246432337 100644
--- a/spp_attachment_av_scan/readme/HISTORY.md
+++ b/spp_attachment_av_scan/readme/HISTORY.md
@@ -1,3 +1,10 @@
+### 19.0.2.0.1
+
+- fix: re-raise database errors (`psycopg2.Error`, `ConcurrencyError`) from the
+ create/write scan-queue hooks instead of swallowing them, so transient
+ serialization failures reach Odoo's transaction retry machinery instead of
+ poisoning the transaction for unrelated downstream code
+
### 19.0.2.0.0
- Initial migration to OpenSPP2
diff --git a/spp_attachment_av_scan/static/description/index.html b/spp_attachment_av_scan/static/description/index.html
index 2b113b66f..426e71d7e 100644
--- a/spp_attachment_av_scan/static/description/index.html
+++ b/spp_attachment_av_scan/static/description/index.html
@@ -515,6 +515,16 @@
+
19.0.2.0.1
+
+- fix: re-raise database errors (psycopg2.Error,
+ConcurrencyError) from the create/write scan-queue hooks instead
+of swallowing them, so transient serialization failures reach Odoo’s
+transaction retry machinery instead of poisoning the transaction for
+unrelated downstream code
+
+
19.0.2.0.0
- Initial migration to OpenSPP2
diff --git a/spp_attachment_av_scan/tests/__init__.py b/spp_attachment_av_scan/tests/__init__.py
index 9ccd65a65..146fb74f3 100644
--- a/spp_attachment_av_scan/tests/__init__.py
+++ b/spp_attachment_av_scan/tests/__init__.py
@@ -1,2 +1,3 @@
from . import test_av_scanner_backend
from . import test_ir_attachment
+from . import test_scan_queue_error_handling
diff --git a/spp_attachment_av_scan/tests/test_scan_queue_error_handling.py b/spp_attachment_av_scan/tests/test_scan_queue_error_handling.py
new file mode 100644
index 000000000..5e2eb6625
--- /dev/null
+++ b/spp_attachment_av_scan/tests/test_scan_queue_error_handling.py
@@ -0,0 +1,131 @@
+"""Queueing a malware scan is best-effort; swallowing a DB error is not.
+
+Incident this pins (dev payroll instance, 2026-07-31): a routine attachment write
+during a module upgrade hit a transient
+
+ ERROR: could not serialize access due to concurrent update
+
+on ``ir_attachment``. The ``create``/``write`` hooks in this module caught it with a
+bare ``except Exception``, logged it, and continued. The transaction was already
+unusable, so the next statement to touch the database — an unrelated
+``env.ref("stock.menu_stock_root")`` inside OpenSPP's menu-icon refresh — failed with
+``InFailedSqlTransaction``, and *that* was the only error the operator ever saw.
+Every module upgrade failed identically, with four different modules blamed in turn.
+
+The swallow also defeated the recovery Odoo already provides:
+``odoo.service.model.retrying`` retries a request on ``IntegrityError`` /
+``OperationalError`` / ``ConcurrencyError``, and ``SerializationFailure`` reaches that
+tuple through ``TransactionRollbackError -> OperationalError``. Left alone, the
+conflict would have been retried transparently. Caught, it became a hard failure
+attributed to the wrong subsystem.
+
+So the contract is two-sided, and both sides are asserted here:
+
+* a **database** error must propagate — it is recoverable, but only if it is allowed
+ to reach the retry machinery;
+* a **non-database** error must still be swallowed and logged — enqueueing a scan is
+ genuinely best-effort and must not block the attachment write.
+"""
+
+import base64
+from unittest.mock import patch
+
+import psycopg2
+
+from odoo.exceptions import ConcurrencyError
+from odoo.service import model as service_model
+from odoo.tests import TransactionCase, tagged
+
+LOGGER = "odoo.addons.spp_attachment_av_scan.models.ir_attachment"
+
+
+def _raise_serialization_failure(*args, **kwargs):
+ """The exact error class from the incident."""
+ raise psycopg2.errors.SerializationFailure("could not serialize access due to concurrent update")
+
+
+def _raise_concurrency_error(*args, **kwargs):
+ """The other member of ``_MUST_NOT_SWALLOW`` — Odoo's own concurrency check."""
+ raise ConcurrencyError("write concurrency check failed")
+
+
+def _raise_value_error(*args, **kwargs):
+ """A non-database failure, e.g. a misconfigured queue channel."""
+ raise ValueError("scan queue is misconfigured")
+
+
+@tagged("post_install", "-at_install")
+class TestScanQueueErrorHandling(TransactionCase):
+ @classmethod
+ def setUpClass(cls):
+ super().setUpClass()
+ cls.Attachment = cls.env["ir.attachment"]
+
+ def _binary_vals(self, name="av-guard-probe.txt", payload=b"probe"):
+ return {
+ "name": name,
+ "datas": base64.b64encode(payload),
+ "mimetype": "text/plain",
+ }
+
+ def test_a_database_error_while_queueing_propagates_on_create(self):
+ """``create`` must not swallow a serialization failure."""
+ with patch.object(type(self.Attachment), "with_delay", _raise_serialization_failure):
+ with self.assertRaises(psycopg2.errors.SerializationFailure):
+ self.Attachment.create(self._binary_vals())
+
+ def test_a_database_error_while_queueing_propagates_on_write(self):
+ """``write`` is the site the incident actually went through."""
+ attachment = self.Attachment.create(self._binary_vals(name="av-guard-write.txt"))
+ with patch.object(type(self.Attachment), "with_delay", _raise_serialization_failure):
+ with self.assertRaises(psycopg2.errors.SerializationFailure):
+ attachment.write({"datas": base64.b64encode(b"changed")})
+
+ def test_a_concurrency_error_while_queueing_propagates_on_create(self):
+ """``ConcurrencyError`` is the second member of ``_MUST_NOT_SWALLOW``."""
+ with patch.object(type(self.Attachment), "with_delay", _raise_concurrency_error):
+ with self.assertRaises(ConcurrencyError):
+ self.Attachment.create(self._binary_vals(name="av-guard-concurrency.txt"))
+
+ def test_a_non_database_error_is_still_swallowed_on_create(self):
+ """Anti-vacuity: the fix must not turn best-effort queueing into a hard gate.
+
+ Re-raising everything would pass both tests above while making any queue
+ misconfiguration block attachment creation across the platform.
+ """
+ with patch.object(type(self.Attachment), "with_delay", _raise_value_error):
+ with self.assertLogs(LOGGER, "ERROR") as logs:
+ attachment = self.Attachment.create(self._binary_vals(name="av-guard-nondb.txt"))
+ self.assertTrue(attachment.exists(), "the attachment must still be created")
+ self.assertIn("Failed to queue malware scan", logs.output[0])
+
+ def test_a_non_database_error_is_still_swallowed_on_write(self):
+ attachment = self.Attachment.create(self._binary_vals(name="av-guard-nondb-write.txt"))
+ with patch.object(type(self.Attachment), "with_delay", _raise_value_error):
+ with self.assertLogs(LOGGER, "ERROR") as logs:
+ attachment.write({"datas": base64.b64encode(b"changed")})
+ self.assertIn("Failed to queue malware scan", logs.output[0])
+
+ def test_the_retry_machinery_can_see_the_error_class_we_re_raise(self):
+ """Guards the *reason* re-raising works, not just that it happens.
+
+ If a future refactor re-raised some wrapped exception instead, the request
+ would no longer be retried and the incident would recur in a new disguise.
+ ``odoo/service/model.py`` retries on ``(IntegrityError, OperationalError,
+ ConcurrencyError)``, then only re-runs the request when the exception is in
+ ``PG_CONCURRENCY_EXCEPTIONS_TO_RETRY``.
+ """
+ self.assertTrue(
+ issubclass(
+ psycopg2.errors.SerializationFailure,
+ (psycopg2.IntegrityError, psycopg2.OperationalError),
+ ),
+ "SerializationFailure must remain catchable by service.model.retrying",
+ )
+ self.assertTrue(
+ issubclass(
+ psycopg2.errors.SerializationFailure,
+ service_model.PG_CONCURRENCY_EXCEPTIONS_TO_RETRY,
+ ),
+ "SerializationFailure must remain in Odoo's retry-eligible set (PG_CONCURRENCY_EXCEPTIONS_TO_RETRY)",
+ )