From 8b9f762c45c204e7bc49815071c0875e05375942 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ha=C5=82as?= Date: Sat, 2 May 2026 23:14:15 +0200 Subject: [PATCH 1/2] fix(SchedulerWorker): log exceptions in child process instead of swallowing When a scheduled task throws an exception in a forked child process, the exception was silently swallowed. The parent only saw exit code 1 with no diagnostic information. Now the exception is logged via Worker::log() before exiting, providing: - task name - exception message - file and line where the exception occurred Fixes #157 --- src/Worker/SchedulerWorker.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Worker/SchedulerWorker.php b/src/Worker/SchedulerWorker.php index 1a369d2..4007d0f 100644 --- a/src/Worker/SchedulerWorker.php +++ b/src/Worker/SchedulerWorker.php @@ -152,7 +152,15 @@ private function runCallback(TriggerInterface $trigger, string $service, string $childExitCode = 0; try { ($this->handler)($service, $taskName); - } catch (\Throwable) { + } catch (\Throwable $e) { + $this->worker->log(sprintf( + '%s Task "%s" failed: %s in %s:%d', + $this->worker->name, + $taskName, + $e->getMessage(), + $e->getFile(), + $e->getLine(), + )); $childExitCode = 1; } finally { // Release lock, cleanup and exit From 0d6cf253b63deeeda3189636fcb390ffe36b370e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ha=C5=82as?= Date: Sat, 2 May 2026 23:23:24 +0200 Subject: [PATCH 2/2] fix(SchedulerWorker): improve exception logging and add test - Add exception type ($e::class) to log message for quick identification - Add stack trace ($e->getTraceAsString()) for full diagnostic info - Add structural test verifying exception logging pattern Addresses PR review comments on #178 --- src/Worker/SchedulerWorker.php | 4 ++- tests/SchedulerWorkerSigchldTest.php | 43 ++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/src/Worker/SchedulerWorker.php b/src/Worker/SchedulerWorker.php index 4007d0f..f33071a 100644 --- a/src/Worker/SchedulerWorker.php +++ b/src/Worker/SchedulerWorker.php @@ -154,12 +154,14 @@ private function runCallback(TriggerInterface $trigger, string $service, string ($this->handler)($service, $taskName); } catch (\Throwable $e) { $this->worker->log(sprintf( - '%s Task "%s" failed: %s in %s:%d', + '%s Task "%s" failed: [%s] %s in %s:%d\nStack trace:\n%s', $this->worker->name, $taskName, + $e::class, $e->getMessage(), $e->getFile(), $e->getLine(), + $e->getTraceAsString(), )); $childExitCode = 1; } finally { diff --git a/tests/SchedulerWorkerSigchldTest.php b/tests/SchedulerWorkerSigchldTest.php index 3beb0b9..47f3fe6 100644 --- a/tests/SchedulerWorkerSigchldTest.php +++ b/tests/SchedulerWorkerSigchldTest.php @@ -87,6 +87,49 @@ public function testSchedulerWorkerUsesCorrectSignalPattern(): void ); } + /** + * Structural test: verify SchedulerWorker logs exceptions in child process. + * This is a regression safety net for Issue #157 — if someone removes the + * exception logging or reverts to silently swallowing exceptions, this test + * catches it immediately. + */ + public function testSchedulerWorkerLogsExceptionsInChildProcess(): void + { + $sourceFile = dirname(__DIR__) . '/src/Worker/SchedulerWorker.php'; + $this->assertFileExists($sourceFile); + + $content = file_get_contents($sourceFile); + $this->assertNotFalse($content); + + // Verify the catch block captures the exception variable + $this->assertStringContainsString( + 'catch (\Throwable $e)', + $content, + 'Catch block must capture exception variable for logging (Issue #157)', + ); + + // Verify the exception is logged via Worker::log() + $this->assertStringContainsString( + '$this->worker->log(sprintf(', + $content, + 'Exception must be logged via Worker::log() in child process (Issue #157)', + ); + + // Verify the log message includes exception type + $this->assertStringContainsString( + '$e::class', + $content, + 'Log message must include exception type for quick identification (Issue #157)', + ); + + // Verify the log message includes stack trace + $this->assertStringContainsString( + '$e->getTraceAsString()', + $content, + 'Log message must include stack trace for full diagnostic information (Issue #157)', + ); + } + /** * Run a SIGCHLD test in an isolated PHP process to avoid PHPUnit * state inheritance issues with pcntl_fork().