mirror of
https://github.com/tiennm99/coolify.git
synced 2026-04-17 19:21:36 +00:00
fix(deployment): improve error logging with exception types and hidden technical details
- Add exception class names to error messages for better debugging - Mark technical details (error type, code, location, stack trace) as hidden in logs - Preserve original exception types when wrapping in DeploymentException - Update ServerManagerJob to include exception class in log messages - Enhance unit tests to verify hidden log entry behavior 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -4,7 +4,6 @@ use App\Exceptions\DeploymentException;
|
||||
use App\Jobs\ApplicationDeploymentJob;
|
||||
use App\Models\Application;
|
||||
use App\Models\ApplicationDeploymentQueue;
|
||||
use Mockery;
|
||||
|
||||
/**
|
||||
* Test to verify that deployment errors are properly logged with comprehensive details.
|
||||
@@ -33,8 +32,8 @@ it('logs comprehensive error details when failed() is called', function () {
|
||||
|
||||
// Capture all log entries
|
||||
$mockQueue->shouldReceive('addLogEntry')
|
||||
->withArgs(function ($message, $type = 'stdout') use (&$logEntries) {
|
||||
$logEntries[] = ['message' => $message, 'type' => $type];
|
||||
->withArgs(function ($message, $type = 'stdout', $hidden = false) use (&$logEntries) {
|
||||
$logEntries[] = ['message' => $message, 'type' => $type, 'hidden' => $hidden];
|
||||
|
||||
return true;
|
||||
})
|
||||
@@ -47,6 +46,9 @@ it('logs comprehensive error details when failed() is called', function () {
|
||||
$mockApplication->shouldReceive('getAttribute')
|
||||
->with('build_pack')
|
||||
->andReturn('dockerfile');
|
||||
$mockApplication->shouldReceive('setAttribute')
|
||||
->with('build_pack', 'dockerfile')
|
||||
->andReturnSelf();
|
||||
$mockApplication->build_pack = 'dockerfile';
|
||||
|
||||
$mockSettings = Mockery::mock();
|
||||
@@ -56,6 +58,8 @@ it('logs comprehensive error details when failed() is called', function () {
|
||||
$mockSettings->shouldReceive('getAttribute')
|
||||
->with('custom_internal_name')
|
||||
->andReturn('');
|
||||
$mockSettings->shouldReceive('setAttribute')
|
||||
->andReturnSelf();
|
||||
$mockSettings->is_consistent_container_name_enabled = false;
|
||||
$mockSettings->custom_internal_name = '';
|
||||
|
||||
@@ -67,7 +71,7 @@ it('logs comprehensive error details when failed() is called', function () {
|
||||
$job = Mockery::mock(ApplicationDeploymentJob::class)->makePartial();
|
||||
$job->shouldAllowMockingProtectedMethods();
|
||||
|
||||
$reflection = new \ReflectionClass($job);
|
||||
$reflection = new \ReflectionClass(ApplicationDeploymentJob::class);
|
||||
|
||||
$queueProperty = $reflection->getProperty('application_deployment_queue');
|
||||
$queueProperty->setAccessible(true);
|
||||
@@ -81,6 +85,10 @@ it('logs comprehensive error details when failed() is called', function () {
|
||||
$pullRequestProperty->setAccessible(true);
|
||||
$pullRequestProperty->setValue($job, 0);
|
||||
|
||||
$containerNameProperty = $reflection->getProperty('container_name');
|
||||
$containerNameProperty->setAccessible(true);
|
||||
$containerNameProperty->setValue($job, 'test-container');
|
||||
|
||||
// Mock the failDeployment method to prevent errors
|
||||
$job->shouldReceive('failDeployment')->andReturn();
|
||||
$job->shouldReceive('execute_remote_command')->andReturn();
|
||||
@@ -106,6 +114,26 @@ it('logs comprehensive error details when failed() is called', function () {
|
||||
// Verify stderr type is used for error logging
|
||||
$stderrEntries = array_filter($logEntries, fn ($entry) => $entry['type'] === 'stderr');
|
||||
expect(count($stderrEntries))->toBeGreaterThan(0);
|
||||
|
||||
// Verify that the main error message is NOT hidden
|
||||
$mainErrorEntry = collect($logEntries)->first(fn ($entry) => str_contains($entry['message'], 'Deployment failed: Failed to start container'));
|
||||
expect($mainErrorEntry['hidden'])->toBeFalse();
|
||||
|
||||
// Verify that technical details ARE hidden
|
||||
$errorTypeEntry = collect($logEntries)->first(fn ($entry) => str_contains($entry['message'], 'Error type:'));
|
||||
expect($errorTypeEntry['hidden'])->toBeTrue();
|
||||
|
||||
$errorCodeEntry = collect($logEntries)->first(fn ($entry) => str_contains($entry['message'], 'Error code:'));
|
||||
expect($errorCodeEntry['hidden'])->toBeTrue();
|
||||
|
||||
$locationEntry = collect($logEntries)->first(fn ($entry) => str_contains($entry['message'], 'Location:'));
|
||||
expect($locationEntry['hidden'])->toBeTrue();
|
||||
|
||||
$stackTraceEntry = collect($logEntries)->first(fn ($entry) => str_contains($entry['message'], 'Stack trace'));
|
||||
expect($stackTraceEntry['hidden'])->toBeTrue();
|
||||
|
||||
$causedByEntry = collect($logEntries)->first(fn ($entry) => str_contains($entry['message'], 'Caused by:'));
|
||||
expect($causedByEntry['hidden'])->toBeTrue();
|
||||
});
|
||||
|
||||
it('handles exceptions with no message gracefully', function () {
|
||||
@@ -115,8 +143,8 @@ it('handles exceptions with no message gracefully', function () {
|
||||
$logEntries = [];
|
||||
|
||||
$mockQueue->shouldReceive('addLogEntry')
|
||||
->withArgs(function ($message, $type = 'stdout') use (&$logEntries) {
|
||||
$logEntries[] = ['message' => $message, 'type' => $type];
|
||||
->withArgs(function ($message, $type = 'stdout', $hidden = false) use (&$logEntries) {
|
||||
$logEntries[] = ['message' => $message, 'type' => $type, 'hidden' => $hidden];
|
||||
|
||||
return true;
|
||||
})
|
||||
@@ -128,6 +156,9 @@ it('handles exceptions with no message gracefully', function () {
|
||||
$mockApplication->shouldReceive('getAttribute')
|
||||
->with('build_pack')
|
||||
->andReturn('dockerfile');
|
||||
$mockApplication->shouldReceive('setAttribute')
|
||||
->with('build_pack', 'dockerfile')
|
||||
->andReturnSelf();
|
||||
$mockApplication->build_pack = 'dockerfile';
|
||||
|
||||
$mockSettings = Mockery::mock();
|
||||
@@ -137,6 +168,8 @@ it('handles exceptions with no message gracefully', function () {
|
||||
$mockSettings->shouldReceive('getAttribute')
|
||||
->with('custom_internal_name')
|
||||
->andReturn('');
|
||||
$mockSettings->shouldReceive('setAttribute')
|
||||
->andReturnSelf();
|
||||
$mockSettings->is_consistent_container_name_enabled = false;
|
||||
$mockSettings->custom_internal_name = '';
|
||||
|
||||
@@ -147,7 +180,7 @@ it('handles exceptions with no message gracefully', function () {
|
||||
$job = Mockery::mock(ApplicationDeploymentJob::class)->makePartial();
|
||||
$job->shouldAllowMockingProtectedMethods();
|
||||
|
||||
$reflection = new \ReflectionClass($job);
|
||||
$reflection = new \ReflectionClass(ApplicationDeploymentJob::class);
|
||||
|
||||
$queueProperty = $reflection->getProperty('application_deployment_queue');
|
||||
$queueProperty->setAccessible(true);
|
||||
@@ -161,6 +194,10 @@ it('handles exceptions with no message gracefully', function () {
|
||||
$pullRequestProperty->setAccessible(true);
|
||||
$pullRequestProperty->setValue($job, 0);
|
||||
|
||||
$containerNameProperty = $reflection->getProperty('container_name');
|
||||
$containerNameProperty->setAccessible(true);
|
||||
$containerNameProperty->setValue($job, 'test-container');
|
||||
|
||||
$job->shouldReceive('failDeployment')->andReturn();
|
||||
$job->shouldReceive('execute_remote_command')->andReturn();
|
||||
|
||||
@@ -197,8 +234,8 @@ it('logs error code 0 correctly', function () {
|
||||
$logEntries = [];
|
||||
|
||||
$mockQueue->shouldReceive('addLogEntry')
|
||||
->withArgs(function ($message, $type = 'stdout') use (&$logEntries) {
|
||||
$logEntries[] = ['message' => $message, 'type' => $type];
|
||||
->withArgs(function ($message, $type = 'stdout', $hidden = false) use (&$logEntries) {
|
||||
$logEntries[] = ['message' => $message, 'type' => $type, 'hidden' => $hidden];
|
||||
|
||||
return true;
|
||||
})
|
||||
@@ -210,6 +247,9 @@ it('logs error code 0 correctly', function () {
|
||||
$mockApplication->shouldReceive('getAttribute')
|
||||
->with('build_pack')
|
||||
->andReturn('dockerfile');
|
||||
$mockApplication->shouldReceive('setAttribute')
|
||||
->with('build_pack', 'dockerfile')
|
||||
->andReturnSelf();
|
||||
$mockApplication->build_pack = 'dockerfile';
|
||||
|
||||
$mockSettings = Mockery::mock();
|
||||
@@ -219,6 +259,8 @@ it('logs error code 0 correctly', function () {
|
||||
$mockSettings->shouldReceive('getAttribute')
|
||||
->with('custom_internal_name')
|
||||
->andReturn('');
|
||||
$mockSettings->shouldReceive('setAttribute')
|
||||
->andReturnSelf();
|
||||
$mockSettings->is_consistent_container_name_enabled = false;
|
||||
$mockSettings->custom_internal_name = '';
|
||||
|
||||
@@ -229,7 +271,7 @@ it('logs error code 0 correctly', function () {
|
||||
$job = Mockery::mock(ApplicationDeploymentJob::class)->makePartial();
|
||||
$job->shouldAllowMockingProtectedMethods();
|
||||
|
||||
$reflection = new \ReflectionClass($job);
|
||||
$reflection = new \ReflectionClass(ApplicationDeploymentJob::class);
|
||||
|
||||
$queueProperty = $reflection->getProperty('application_deployment_queue');
|
||||
$queueProperty->setAccessible(true);
|
||||
@@ -243,6 +285,10 @@ it('logs error code 0 correctly', function () {
|
||||
$pullRequestProperty->setAccessible(true);
|
||||
$pullRequestProperty->setValue($job, 0);
|
||||
|
||||
$containerNameProperty = $reflection->getProperty('container_name');
|
||||
$containerNameProperty->setAccessible(true);
|
||||
$containerNameProperty->setValue($job, 'test-container');
|
||||
|
||||
$job->shouldReceive('failDeployment')->andReturn();
|
||||
$job->shouldReceive('execute_remote_command')->andReturn();
|
||||
|
||||
@@ -256,3 +302,43 @@ it('logs error code 0 correctly', function () {
|
||||
// Should log error code 0 (not skip it)
|
||||
expect($errorMessageString)->toContain('Error code: 0');
|
||||
});
|
||||
|
||||
it('preserves original exception type in wrapped DeploymentException messages', function () {
|
||||
// Verify that when wrapping exceptions, the original exception type is included in the message
|
||||
$originalException = new \RuntimeException('Connection timeout');
|
||||
|
||||
// Test rolling update scenario
|
||||
$wrappedException = new DeploymentException(
|
||||
'Rolling update failed ('.get_class($originalException).'): '.$originalException->getMessage(),
|
||||
$originalException->getCode(),
|
||||
$originalException
|
||||
);
|
||||
|
||||
expect($wrappedException->getMessage())->toContain('RuntimeException');
|
||||
expect($wrappedException->getMessage())->toContain('Connection timeout');
|
||||
expect($wrappedException->getPrevious())->toBe($originalException);
|
||||
|
||||
// Test health check scenario
|
||||
$healthCheckException = new \InvalidArgumentException('Invalid health check URL');
|
||||
$wrappedHealthCheck = new DeploymentException(
|
||||
'Health check failed ('.get_class($healthCheckException).'): '.$healthCheckException->getMessage(),
|
||||
$healthCheckException->getCode(),
|
||||
$healthCheckException
|
||||
);
|
||||
|
||||
expect($wrappedHealthCheck->getMessage())->toContain('InvalidArgumentException');
|
||||
expect($wrappedHealthCheck->getMessage())->toContain('Invalid health check URL');
|
||||
expect($wrappedHealthCheck->getPrevious())->toBe($healthCheckException);
|
||||
|
||||
// Test docker registry push scenario
|
||||
$registryException = new \RuntimeException('Failed to authenticate');
|
||||
$wrappedRegistry = new DeploymentException(
|
||||
get_class($registryException).': '.$registryException->getMessage(),
|
||||
$registryException->getCode(),
|
||||
$registryException
|
||||
);
|
||||
|
||||
expect($wrappedRegistry->getMessage())->toContain('RuntimeException');
|
||||
expect($wrappedRegistry->getMessage())->toContain('Failed to authenticate');
|
||||
expect($wrappedRegistry->getPrevious())->toBe($registryException);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user