From e5569f681adbd14f254e4e594d064628cc1e8ff0 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Thu, 2 Jul 2026 00:52:20 +0530 Subject: [PATCH] fix: classify repost recovery by exception type, not traceback string repost() decided whether a failed Repost Item Valuation was recoverable (re-queue as "In Progress") or permanently "Failed" by string-matching the traceback for "timeout" or MariaDB's "Deadlock found". On Postgres a deadlock surfaces as "deadlock detected" / "could not serialize access" and matches neither, so a retriable deadlock was marked Failed and never re-queued -- the scheduler only re-picks Queued/In Progress entries. Classify by isinstance(e, RecoverableErrors) instead, the same tuple already used to gate the error email. This covers deadlocks and lock/query timeouts on both engines (frappe raises QueryDeadlockError / QueryTimeoutError uniformly) and the advisory-lock repost gate's own QueryTimeoutError, which previously recovered only because its class name incidentally contains "timeout". --- .../repost_item_valuation.py | 11 ++++--- .../test_repost_item_valuation.py | 33 +++++++++++++++++++ 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py b/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py index b9bb3d931da..e113b8fe464 100644 --- a/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py +++ b/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py @@ -425,10 +425,10 @@ def repost(doc): if isinstance(message, dict): message = message.get("message") - status = "Failed" - # If failed because of timeout, set status to In Progress - if traceback and ("timeout" in traceback.lower() or "Deadlock found" in traceback): - status = "In Progress" + # Recoverable errors (deadlock, lock/query timeout, job timeout) re-queue as In Progress. + # Classify by type: the old traceback string-match only knew MariaDB's "Deadlock found" and + # missed Postgres deadlocks ("deadlock detected"), failing them permanently. + status = "In Progress" if isinstance(e, RecoverableErrors) else "Failed" if traceback: message += "

" + "Traceback:
" + traceback @@ -447,7 +447,8 @@ def repost(doc): "Email Account", {"default_outgoing": 1, "enable_outgoing": 1}, "name" ) - if outgoing_email_account and not isinstance(e, RecoverableErrors): + # status == "Failed" already implies e is not recoverable, so no need to re-check here. + if outgoing_email_account: notify_error_to_stock_managers(doc, message) doc.set_status("Failed") finally: diff --git a/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py b/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py index 40df17a061d..d2c5eaa6096 100644 --- a/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py +++ b/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py @@ -220,6 +220,39 @@ class TestRepostItemValuation(ERPNextTestSuite, StockTestMixin): sorted(frappe.parse_json(frappe.as_json(set([("a", "b"), ("c", "d")])))), ) + def test_recoverable_error_requeues_instead_of_failing(self): + # A recoverable DB error (e.g. Postgres deadlock -> QueryDeadlockError) must re-queue the + # repost as "In Progress"; a non-recoverable error still fails. Regression: the old check + # string-matched MariaDB's "Deadlock found" and missed Postgres deadlocks ("deadlock detected"). + from unittest.mock import patch + + from frappe.exceptions import QueryDeadlockError + + from erpnext.stock.doctype.repost_item_valuation import repost_item_valuation as riv + + orig_max_writes = frappe.db.MAX_WRITES_PER_TRANSACTION + self.addCleanup(setattr, frappe.db, "MAX_WRITES_PER_TRANSACTION", orig_max_writes) + + def status_after(error): + doc = frappe.new_doc("Repost Item Valuation") + doc.name = "test-recoverable-riv" + doc.set_status = doc.log_error = doc.db_set = MagicMock() + captured = {} + with ( + patch.object(frappe, "in_test", False), + patch.object(frappe.db, "exists", return_value=True), + patch.object(frappe.db, "commit"), + patch.object(frappe.db, "rollback"), + patch.object(frappe.db, "set_value", side_effect=lambda *a, **k: captured.update(a[2])), + patch.object(riv, "repost_sl_entries", side_effect=error), + patch.object(frappe, "get_cached_value", return_value=None), + ): + riv.repost(doc) + return captured.get("status") + + self.assertEqual(status_after(QueryDeadlockError("deadlock detected")), "In Progress") + self.assertEqual(status_after(ValueError("boom")), "Failed") + def test_gl_repost_progress(self): from erpnext.accounts import utils