From 80371d48fbf5c9924e1b90502f3b57bf0bb7e7f5 Mon Sep 17 00:00:00 2001
From: Patrick Niebeling <patrick@niebel.ing>
Date: Thu, 13 Aug 2026 15:24:16 +0200
Subject: [PATCH] Never let a throwable escape the workflow run

Job::start() calls setLastRun() before run() and clears `reserved_at` only
afterwards through setExecutionTime(), which is not in a finally block. Any
uncaught throwable therefore leaves the job reserved, and JobList::getNext()
skips reserved jobs until the reservation is 12 hours stale -- so a single
failing workflow takes the whole app offline for half a day, while last_run
still shows the start of that failed attempt.

Wrap both loops so a failure is logged with its exception and the remaining
users and workflows still run.
---
 lib/Service/WorkflowRunner.php | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/lib/Service/WorkflowRunner.php b/lib/Service/WorkflowRunner.php
index 94228ff..9f58922 100644
--- a/lib/Service/WorkflowRunner.php
+++ b/lib/Service/WorkflowRunner.php
@@ -52,7 +52,21 @@ class WorkflowRunner {
 		}
 
 		foreach ($byUser as $userId => $userWorkflows) {
-			$this->runForUser((string)$userId, $userWorkflows);
+			// Nothing may escape this loop. Job::start() calls setLastRun()
+			// *before* run() and only clears `reserved_at` afterwards via
+			// setExecutionTime(), which is not in a finally — so a single
+			// uncaught throwable leaves the job reserved, and JobList::getNext()
+			// then skips it until the reservation is 12 hours stale. One bad
+			// workflow would silently take the whole app offline for half a day.
+			try {
+				$this->runForUser((string)$userId, $userWorkflows);
+			} catch (Throwable $e) {
+				$this->logger->error('Workflow run failed for {user}: ' . $e->getMessage(), [
+					'app' => 'workflow_deck_automation',
+					'user' => $userId,
+					'exception' => $e,
+				]);
+			}
 		}
 	}
 
@@ -78,7 +92,17 @@ class WorkflowRunner {
 		$this->impersonate($user);
 		try {
 			foreach ($workflows as $workflow) {
-				$this->runWorkflow($user, $workflow);
+				// Same reasoning as in run(): one broken workflow must not
+				// stop the user's remaining ones.
+				try {
+					$this->runWorkflow($user, $workflow);
+				} catch (Throwable $e) {
+					$this->logger->error('Workflow {id} failed: ' . $e->getMessage(), [
+						'app' => 'workflow_deck_automation',
+						'id' => $workflow->getId(),
+						'exception' => $e,
+					]);
+				}
 			}
 		} finally {
 			$this->clearImpersonation();