From 7904ca9a5c59ceb9643e3840452f2310d1ead557 Mon Sep 17 00:00:00 2001 From: Shariq Khatri Date: Thu, 5 Nov 2026 14:03:36 +0530 Subject: [PATCH] fix(agent-job): Tell the team to remove folders from private/backups Frappe deletes old files in private/backups before each backup. If a folder is in private/backups, os.remove raises IsADirectoryError and the backup fails. The team saw only "Job Failure" and did not know the cause, because a custom app or a script made the folder. The banner on the failed job now names the folder and tells the team to remove it. It has no Help link, because no doc page covers this error. The real fix is in frappe (skip folders), but sites on older versions continue to fail until then. Co-Authored-By: Claude Opus 5.5 --- .../agent_job/agent_job_notifications.py | 24 +++++++++++++ .../press/doctype/agent_job/test_agent_job.py | 35 +++++++++++++++++++ 2 files changed, 59 insertions(+) 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)