From 64c7d301ce7b7098c94343c474f23720ddf51bab Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 11 Nov 2025 12:07:35 +0100 Subject: [PATCH] feat(DatabaseBackupJob, ScheduledTaskJob): enforce minimum timeout and add execution ID for timeout handling --- app/Jobs/DatabaseBackupJob.php | 2 +- app/Jobs/ScheduledTaskJob.php | 48 +++++++++- app/Livewire/Project/Database/BackupEdit.php | 2 +- tests/Unit/ScheduledJobsRetryConfigTest.php | 23 ++++- tests/Unit/ScheduledTaskJobTimeoutTest.php | 96 ++++++++++++++++++++ 5 files changed, 165 insertions(+), 6 deletions(-) create mode 100644 tests/Unit/ScheduledTaskJobTimeoutTest.php diff --git a/app/Jobs/DatabaseBackupJob.php b/app/Jobs/DatabaseBackupJob.php index ff16c78e0..b28bce7cf 100644 --- a/app/Jobs/DatabaseBackupJob.php +++ b/app/Jobs/DatabaseBackupJob.php @@ -85,7 +85,7 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue public function __construct(public ScheduledDatabaseBackup $backup) { $this->onQueue('high'); - $this->timeout = $backup->timeout; + $this->timeout = $backup->timeout ?? 3600; } public function handle(): void diff --git a/app/Jobs/ScheduledTaskJob.php b/app/Jobs/ScheduledTaskJob.php index 41a6ec8e2..1776d0d78 100644 --- a/app/Jobs/ScheduledTaskJob.php +++ b/app/Jobs/ScheduledTaskJob.php @@ -49,6 +49,11 @@ class ScheduledTaskJob implements ShouldQueue public ?ScheduledTaskExecution $task_log = null; + /** + * Store execution ID to survive job serialization for timeout handling. + */ + protected ?int $executionId = null; + public string $task_status = 'failed'; public ?string $task_output = null; @@ -98,6 +103,9 @@ public function handle(): void 'retry_count' => $this->attempts() - 1, ]); + // Store execution ID for timeout handling + $this->executionId = $this->task_log->id; + $this->server = $this->resource->destination->server; if ($this->resource->type() === 'application') { @@ -204,12 +212,46 @@ public function failed(?\Throwable $exception): void 'trace' => $exception?->getTraceAsString(), ]); - if ($this->task_log) { - $this->task_log->update([ + // Reload execution log from database + // When a job times out, failed() is called in a fresh process with the original + // queue payload, so $executionId will be null. We need to query for the latest execution. + $execution = null; + + // Try to find execution using stored ID first (works for non-timeout failures) + if ($this->executionId) { + $execution = ScheduledTaskExecution::find($this->executionId); + } + + // If no stored ID or not found, query for the most recent execution log for this task + if (! $execution) { + $execution = ScheduledTaskExecution::query() + ->where('scheduled_task_id', $this->task->id) + ->orderBy('created_at', 'desc') + ->first(); + } + + // Last resort: check task_log property + if (! $execution && $this->task_log) { + $execution = $this->task_log; + } + + if ($execution) { + $errorMessage = 'Job permanently failed after '.$this->attempts().' attempts'; + if ($exception) { + $errorMessage .= ': '.$exception->getMessage(); + } + + $execution->update([ 'status' => 'failed', - 'message' => 'Job permanently failed after '.$this->attempts().' attempts: '.($exception?->getMessage() ?? 'Unknown error'), + 'message' => $errorMessage, + 'error_details' => $exception?->getTraceAsString(), 'finished_at' => Carbon::now()->toImmutable(), ]); + } else { + Log::channel('scheduled-errors')->warning('Could not find execution log to update', [ + 'execution_id' => $this->executionId, + 'task_id' => $this->task->uuid, + ]); } // Notify team about permanent failure diff --git a/app/Livewire/Project/Database/BackupEdit.php b/app/Livewire/Project/Database/BackupEdit.php index 7deaa82a9..da543a049 100644 --- a/app/Livewire/Project/Database/BackupEdit.php +++ b/app/Livewire/Project/Database/BackupEdit.php @@ -79,7 +79,7 @@ class BackupEdit extends Component #[Validate(['required', 'boolean'])] public bool $dumpAll = false; - #[Validate(['required', 'int', 'min:1', 'max:36000'])] + #[Validate(['required', 'int', 'min:60', 'max:36000'])] public int $timeout = 3600; public function mount() diff --git a/tests/Unit/ScheduledJobsRetryConfigTest.php b/tests/Unit/ScheduledJobsRetryConfigTest.php index bf959a0f5..f46cb9fd1 100644 --- a/tests/Unit/ScheduledJobsRetryConfigTest.php +++ b/tests/Unit/ScheduledJobsRetryConfigTest.php @@ -45,6 +45,7 @@ // Check public properties exist expect($reflection->hasProperty('tries'))->toBeTrue() ->and($reflection->hasProperty('maxExceptions'))->toBeTrue() + ->and($reflection->hasProperty('timeout'))->toBeTrue() ->and($reflection->hasMethod('backoff'))->toBeTrue() ->and($reflection->hasMethod('failed'))->toBeTrue(); @@ -52,5 +53,25 @@ $defaultProperties = $reflection->getDefaultProperties(); expect($defaultProperties['tries'])->toBe(2) - ->and($defaultProperties['maxExceptions'])->toBe(1); + ->and($defaultProperties['maxExceptions'])->toBe(1) + ->and($defaultProperties['timeout'])->toBe(3600); +}); + +it('DatabaseBackupJob enforces minimum timeout of 60 seconds', function () { + // Read the constructor to verify minimum timeout enforcement + $reflection = new ReflectionClass(DatabaseBackupJob::class); + $constructor = $reflection->getMethod('__construct'); + + // Get the constructor source + $filename = $reflection->getFileName(); + $startLine = $constructor->getStartLine(); + $endLine = $constructor->getEndLine(); + + $source = file($filename); + $constructorSource = implode('', array_slice($source, $startLine - 1, $endLine - $startLine + 1)); + + // Verify the implementation enforces minimum of 60 seconds + expect($constructorSource) + ->toContain('max(') + ->toContain('60'); }); diff --git a/tests/Unit/ScheduledTaskJobTimeoutTest.php b/tests/Unit/ScheduledTaskJobTimeoutTest.php new file mode 100644 index 000000000..99117fbca --- /dev/null +++ b/tests/Unit/ScheduledTaskJobTimeoutTest.php @@ -0,0 +1,96 @@ +hasProperty('executionId'))->toBeTrue(); + + // Verify it's protected (will be serialized with the job) + $property = $reflection->getProperty('executionId'); + expect($property->isProtected())->toBeTrue(); +}); + +it('has failed method that handles job failures', function () { + $reflection = new ReflectionClass(ScheduledTaskJob::class); + + // Verify failed() method exists + expect($reflection->hasMethod('failed'))->toBeTrue(); + + // Verify it accepts a Throwable parameter + $method = $reflection->getMethod('failed'); + $parameters = $method->getParameters(); + + expect($parameters)->toHaveCount(1); + expect($parameters[0]->getName())->toBe('exception'); + expect($parameters[0]->allowsNull())->toBeTrue(); +}); + +it('failed method implementation reloads execution from database', function () { + // Read the failed() method source code to verify it reloads from database + $reflection = new ReflectionClass(ScheduledTaskJob::class); + $method = $reflection->getMethod('failed'); + + // Get the file and method source + $filename = $reflection->getFileName(); + $startLine = $method->getStartLine(); + $endLine = $method->getEndLine(); + + $source = file($filename); + $methodSource = implode('', array_slice($source, $startLine - 1, $endLine - $startLine + 1)); + + // Verify the implementation includes reloading from database + expect($methodSource) + ->toContain('$this->executionId') + ->toContain('ScheduledTaskExecution::find') + ->toContain('ScheduledTaskExecution::query') + ->toContain('scheduled_task_id') + ->toContain('orderBy') + ->toContain('status') + ->toContain('failed') + ->toContain('notify'); +}); + +it('failed method updates execution with error_details field', function () { + // Read the failed() method source code to verify error_details is populated + $reflection = new ReflectionClass(ScheduledTaskJob::class); + $method = $reflection->getMethod('failed'); + + // Get the file and method source + $filename = $reflection->getFileName(); + $startLine = $method->getStartLine(); + $endLine = $method->getEndLine(); + + $source = file($filename); + $methodSource = implode('', array_slice($source, $startLine - 1, $endLine - $startLine + 1)); + + // Verify the implementation populates error_details field + expect($methodSource)->toContain('error_details'); +}); + +it('failed method logs when execution cannot be found', function () { + // Read the failed() method source code to verify defensive logging + $reflection = new ReflectionClass(ScheduledTaskJob::class); + $method = $reflection->getMethod('failed'); + + // Get the file and method source + $filename = $reflection->getFileName(); + $startLine = $method->getStartLine(); + $endLine = $method->getEndLine(); + + $source = file($filename); + $methodSource = implode('', array_slice($source, $startLine - 1, $endLine - $startLine + 1)); + + // Verify the implementation logs a warning if execution is not found + expect($methodSource) + ->toContain('Could not find execution log') + ->toContain('warning'); +});