diff --git a/press/press/doctype/agent_job/agent_job_notifications.py b/press/press/doctype/agent_job/agent_job_notifications.py index e997903c986..81eb2a3ab1a 100644 --- a/press/press/doctype/agent_job/agent_job_notifications.py +++ b/press/press/doctype/agent_job/agent_job_notifications.py @@ -36,6 +36,9 @@ class Details(TypedDict): # Bench app directory in a traceback frame, app names are python module names APP_FRAME = re.compile(r"/apps/([a-z_][a-z0-9_]*)/") +# Path in the error that Frappe raises when it tries to delete a folder as an old backup file +BACKUP_FOLDER_PATH = re.compile(r"IsADirectoryError: .*?'([^']*/private/backups/[^']+)'") + if typing.TYPE_CHECKING: # TYPE_CHECKING guard for code below cause DeployCandidate # might cause circular import. @@ -126,6 +129,7 @@ def handlers() -> list[UserAddressableHandlerTuple]: ("Unknown command '\\-'.", update_with_unknown_command_hyphen_err), ('redis_host, redis_port = redis_url.split(":")', update_with_redis_unpack_error), ("Site might have lot of jobs in queue.", update_with_rq_jobs_in_queue_err), + (["IsADirectoryError", "/private/backups/"], update_with_folder_in_backups_err), # Keep last: matches any traceback, only actionable if the failing app is identifiable ("/apps/", update_with_app_failure_err), ] @@ -411,6 +415,26 @@ def update_with_rq_jobs_in_queue_err(details: Details, job: AgentJob): return True +def update_with_folder_in_backups_err(details: Details, job: AgentJob): + """Frappe deletes old files in private/backups before a backup, and fails on a folder""" + details["title"] = "Backup failed because of a folder in the backups folder" + + details["message"] = ( + f"

The backup of site {job.site} failed because of the folder {get_backup_folder_path(job)}.

" + "

Before each backup, Frappe deletes old files in the private/backups folder of the site." + " It cannot delete a folder, so the backup stops.

" + "

Delete all folders in private/backups, or move them to a different location. Then take the backup again." + " If an app or a script makes these folders, change it to use a different location.

" + ) + + return True + + +def get_backup_folder_path(job: AgentJob) -> str: + match = BACKUP_FOLDER_PATH.search(f"{job.traceback or ''}\n{job.output or ''}") + return match.group(1) if match else "private/backups" + + def has_newer_release(bench: str, app: str) -> bool: """False only if we know the bench is on the newest release of the app""" deployed = frappe.db.get_value( diff --git a/press/press/doctype/agent_job/test_agent_job.py b/press/press/doctype/agent_job/test_agent_job.py index 3f724bd7365..b9c198b528c 100644 --- a/press/press/doctype/agent_job/test_agent_job.py +++ b/press/press/doctype/agent_job/test_agent_job.py @@ -644,6 +644,41 @@ def test_disk_full_on_public_server_is_not_actionable(self): self.assertFalse(details["is_actionable"]) + FOLDER_IN_BACKUPS_TRACEBACK = ( + ' File "apps/frappe/frappe/utils/backups.py", line 662, in delete_temp_backups\n' + " os.remove(this_file_path)\n" + "IsADirectoryError: [Errno 21] Is a directory: './test.frappe.dev/private/backups/uom-night-20261102'" + ) + + @patch.object(AgentJob, "enqueue_http_request", new=Mock()) + def test_folder_in_backups_folder_names_the_folder_to_delete(self): + details = get_details(self.backup_job(self.FOLDER_IN_BACKUPS_TRACEBACK), "", "") + + self.assertTrue(details["is_actionable"]) + self.assertEqual(details["title"], "Backup failed because of a folder in the backups folder") + self.assertIn("./test.frappe.dev/private/backups/uom-night-20261102", details["message"]) + + @patch.object(AgentJob, "enqueue_http_request", new=Mock()) + def test_folder_error_outside_backups_folder_is_not_actionable(self): + traceback = "IsADirectoryError: [Errno 21] Is a directory: './test.frappe.dev/public/files/reports'" + + details = get_details(self.backup_job(traceback), "", "") + + self.assertFalse(details["is_actionable"]) + + def backup_job(self, traceback: str) -> AgentJob: + site = create_test_site() + return frappe.get_doc( + { + "doctype": "Agent Job", + "job_type": "Backup Site", + "site": site.name, + "server_type": "Server", + "server": site.server, + "traceback": traceback, + } + ) + def restore_job(self, public: bool) -> AgentJob: site = create_test_site() frappe.db.set_value("Server", site.server, "public", public)