feat(DatabaseBackupJob, ScheduledTaskJob): enforce minimum timeout and add execution ID for timeout handling
This commit is contained in:
parent
104e68a9ac
commit
64c7d301ce
5 changed files with 165 additions and 6 deletions
|
|
@ -85,7 +85,7 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue
|
||||||
public function __construct(public ScheduledDatabaseBackup $backup)
|
public function __construct(public ScheduledDatabaseBackup $backup)
|
||||||
{
|
{
|
||||||
$this->onQueue('high');
|
$this->onQueue('high');
|
||||||
$this->timeout = $backup->timeout;
|
$this->timeout = $backup->timeout ?? 3600;
|
||||||
}
|
}
|
||||||
|
|
||||||
public function handle(): void
|
public function handle(): void
|
||||||
|
|
|
||||||
|
|
@ -49,6 +49,11 @@ class ScheduledTaskJob implements ShouldQueue
|
||||||
|
|
||||||
public ?ScheduledTaskExecution $task_log = null;
|
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_status = 'failed';
|
||||||
|
|
||||||
public ?string $task_output = null;
|
public ?string $task_output = null;
|
||||||
|
|
@ -98,6 +103,9 @@ public function handle(): void
|
||||||
'retry_count' => $this->attempts() - 1,
|
'retry_count' => $this->attempts() - 1,
|
||||||
]);
|
]);
|
||||||
|
|
||||||
|
// Store execution ID for timeout handling
|
||||||
|
$this->executionId = $this->task_log->id;
|
||||||
|
|
||||||
$this->server = $this->resource->destination->server;
|
$this->server = $this->resource->destination->server;
|
||||||
|
|
||||||
if ($this->resource->type() === 'application') {
|
if ($this->resource->type() === 'application') {
|
||||||
|
|
@ -204,12 +212,46 @@ public function failed(?\Throwable $exception): void
|
||||||
'trace' => $exception?->getTraceAsString(),
|
'trace' => $exception?->getTraceAsString(),
|
||||||
]);
|
]);
|
||||||
|
|
||||||
if ($this->task_log) {
|
// Reload execution log from database
|
||||||
$this->task_log->update([
|
// 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',
|
'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(),
|
'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
|
// Notify team about permanent failure
|
||||||
|
|
|
||||||
|
|
@ -79,7 +79,7 @@ class BackupEdit extends Component
|
||||||
#[Validate(['required', 'boolean'])]
|
#[Validate(['required', 'boolean'])]
|
||||||
public bool $dumpAll = false;
|
public bool $dumpAll = false;
|
||||||
|
|
||||||
#[Validate(['required', 'int', 'min:1', 'max:36000'])]
|
#[Validate(['required', 'int', 'min:60', 'max:36000'])]
|
||||||
public int $timeout = 3600;
|
public int $timeout = 3600;
|
||||||
|
|
||||||
public function mount()
|
public function mount()
|
||||||
|
|
|
||||||
|
|
@ -45,6 +45,7 @@
|
||||||
// Check public properties exist
|
// Check public properties exist
|
||||||
expect($reflection->hasProperty('tries'))->toBeTrue()
|
expect($reflection->hasProperty('tries'))->toBeTrue()
|
||||||
->and($reflection->hasProperty('maxExceptions'))->toBeTrue()
|
->and($reflection->hasProperty('maxExceptions'))->toBeTrue()
|
||||||
|
->and($reflection->hasProperty('timeout'))->toBeTrue()
|
||||||
->and($reflection->hasMethod('backoff'))->toBeTrue()
|
->and($reflection->hasMethod('backoff'))->toBeTrue()
|
||||||
->and($reflection->hasMethod('failed'))->toBeTrue();
|
->and($reflection->hasMethod('failed'))->toBeTrue();
|
||||||
|
|
||||||
|
|
@ -52,5 +53,25 @@
|
||||||
$defaultProperties = $reflection->getDefaultProperties();
|
$defaultProperties = $reflection->getDefaultProperties();
|
||||||
|
|
||||||
expect($defaultProperties['tries'])->toBe(2)
|
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');
|
||||||
});
|
});
|
||||||
|
|
|
||||||
96
tests/Unit/ScheduledTaskJobTimeoutTest.php
Normal file
96
tests/Unit/ScheduledTaskJobTimeoutTest.php
Normal file
|
|
@ -0,0 +1,96 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
use App\Jobs\ScheduledTaskJob;
|
||||||
|
use Illuminate\Support\Facades\Log;
|
||||||
|
|
||||||
|
beforeEach(function () {
|
||||||
|
// Mock Log facade to prevent actual logging during tests
|
||||||
|
Log::spy();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('has executionId property for timeout handling', function () {
|
||||||
|
$reflection = new ReflectionClass(ScheduledTaskJob::class);
|
||||||
|
|
||||||
|
// Verify executionId property exists
|
||||||
|
expect($reflection->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');
|
||||||
|
});
|
||||||
Loading…
Reference in a new issue