fix: handle snapshot isolation errors better (#32318)

* fix: Avoid Snapshot violation

- Main thread created and "read" user
- Other thread modified something
- Main thread wants to delete or "write" to same row.

This violates snapshot isolation.

* fix: treat snapshot violation as deadlock for now

* test: handle snapshot violations

frappe/core/doctype/user/test_user.py

@@ -496,6 +496,7 @@ def test_user(
 		user.append_roles(*roles)
 		user.insert()
 		yield user
+		commit and frappe.db.commit()
 	finally:
 		user.delete(force=True, ignore_permissions=True)
 		commit and frappe.db.commit()

frappe/database/mariadb/database.py

@@ -25,7 +25,8 @@ class MariaDBExceptionUtil:
 
 	@staticmethod
 	def is_deadlocked(e: pymysql.Error) -> bool:
-		return e.args[0] == ER.LOCK_DEADLOCK
+		# Snapshot isolation is also treated as deadlock from User POV
+		return e.args[0] in (ER.LOCK_DEADLOCK, ER.CHECKREAD)
 
 	@staticmethod
 	def is_timedout(e: pymysql.Error) -> bool:

frappe/database/mariadb/mysqlclient.py

@@ -28,7 +28,8 @@ class MariaDBExceptionUtil:
 
 	@staticmethod
 	def is_deadlocked(e: MySQLdb.Error) -> bool:
-		return e.args[0] == ER.LOCK_DEADLOCK
+		# Snapshot isolation is also treated as deadlock from User POV
+		return e.args[0] in (ER.LOCK_DEADLOCK, ER.CHECKREAD)
 
 	@staticmethod
 	def is_timedout(e: MySQLdb.Error) -> bool:

frappe/integrations/doctype/connected_app/test_connected_app.py

@@ -137,6 +137,8 @@ class TestConnectedApp(IntegrationTestCase):
 			if doc:
 				doc.delete(force=True)
 
+		frappe.db.commit()  # Avoid snapshot violation issues
+
 		delete_if_exists("token_cache")
 		delete_if_exists("connected_app")
 

frappe/model/delete_doc.py

@@ -107,7 +107,7 @@ def delete_doc(
 			# Lock the doc without waiting
 			try:
 				frappe.db.get_value(doctype, name, for_update=True, wait=False)
-			except frappe.QueryTimeoutError:
+			except (frappe.QueryTimeoutError, frappe.QueryDeadlockError):
 				frappe.throw(
 					_(
 						"This document can not be deleted right now as it's being modified by another user. Please try again after some time."

frappe/public/js/frappe/request.js

@@ -251,7 +251,9 @@ frappe.request.call = function (opts) {
 			frappe.msgprint({
 				title: __("Deadlock Occurred"),
 				indicator: "red",
-				message: __("Server was too busy to process this request. Please try again."),
+				message: __(
+					"Server failed to process this request because of a concurrent conflicting request. Please try again."
+				),
 			});
 		},
 	};

frappe/tests/test_api.py

@@ -130,6 +130,10 @@ class FrappeAPITestCase(IntegrationTestCase):
 	def delete(self, path, **kwargs) -> TestResponse:
 		return make_request(target=self.TEST_CLIENT.delete, args=(path,), kwargs=kwargs)
 
+	def tearDown(self) -> None:
+		frappe.db.rollback()
+		return super().tearDown()
+
 
 class TestResourceAPI(FrappeAPITestCase):
 	DOCTYPE = "ToDo"
@@ -146,6 +150,7 @@ class TestResourceAPI(FrappeAPITestCase):
 
 	@classmethod
 	def tearDownClass(cls):
+		frappe.db.commit()
 		for name in cls.GENERATED_DOCUMENTS:
 			frappe.delete_doc_if_exists(cls.DOCTYPE, name)
 		frappe.db.commit()

frappe/tests/test_api_v2.py

@@ -33,6 +33,7 @@ class TestResourceAPIV2(FrappeAPITestCase):
 
 	@classmethod
 	def tearDownClass(cls):
+		frappe.db.commit()
 		for name in cls.GENERATED_DOCUMENTS:
 			frappe.delete_doc_if_exists(cls.DOCTYPE, name)
 		frappe.db.commit()

frappe/tests/test_auth.py

@@ -49,10 +49,12 @@ class TestAuth(IntegrationTestCase):
 
 	@classmethod
 	def tearDownClass(cls):
+		frappe.db.rollback()
 		frappe.delete_doc("User", cls.test_user_email, force=True)
 		frappe.local.request_ip = None
 		frappe.form_dict.email = None
 		frappe.local.response["http_status_code"] = None
+		frappe.db.commit()
 
 	def set_system_settings(self, k, v):
 		frappe.db.set_single_value("System Settings", k, v)

frappe/tests/test_frappe_client.py

@@ -105,7 +105,9 @@ class TestFrappeClient(IntegrationTestCase):
 		self.assertEqual(
 			server.get_value("Website Settings", "title_prefix").get("title_prefix"), "test-prefix"
 		)
+		frappe.db.rollback()  # Clear snapshot isolation
 		frappe.db.set_single_value("Website Settings", "title_prefix", "")
+		frappe.db.commit()
 
 	def test_update_doc(self):
 		server = FrappeClient(get_url(), "Administrator", self.PASSWORD, verify=False)