fix(s3): cap connection checks at 15 seconds
Return a friendly timeout error for failed S3 endpoint checks while preserving the original exception as the previous throwable.
This commit is contained in:
parent
20f9bb4305
commit
dd8a0d501d
2 changed files with 95 additions and 5 deletions
|
|
@ -14,6 +14,10 @@ class S3Storage extends BaseModel
|
|||
{
|
||||
use HasFactory, HasSafeStringAttribute;
|
||||
|
||||
private const CONNECTION_TIMEOUT_SECONDS = 15;
|
||||
|
||||
private const REQUEST_TIMEOUT_SECONDS = 15;
|
||||
|
||||
protected $fillable = [
|
||||
'name',
|
||||
'description',
|
||||
|
|
@ -157,6 +161,10 @@ public function testConnection(bool $shouldSave = false)
|
|||
'bucket' => $this['bucket'],
|
||||
'endpoint' => $this['endpoint'],
|
||||
'use_path_style_endpoint' => true,
|
||||
'http' => [
|
||||
'connect_timeout' => self::CONNECTION_TIMEOUT_SECONDS,
|
||||
'timeout' => self::REQUEST_TIMEOUT_SECONDS,
|
||||
],
|
||||
]);
|
||||
// Test the connection by listing files with ListObjectsV2 (S3)
|
||||
$disk->files();
|
||||
|
|
@ -164,11 +172,12 @@ public function testConnection(bool $shouldSave = false)
|
|||
$this->unusable_email_sent = false;
|
||||
$this->is_usable = true;
|
||||
} catch (\Throwable $e) {
|
||||
$exception = $this->toUserFriendlyConnectionException($e);
|
||||
$this->is_usable = false;
|
||||
if ($this->unusable_email_sent === false && is_transactional_emails_enabled()) {
|
||||
$mail = new MailMessage;
|
||||
$mail->subject('Coolify: S3 Storage Connection Error');
|
||||
$mail->view('emails.s3-connection-error', ['name' => $this->name, 'reason' => $e->getMessage(), 'url' => route('storage.show', ['storage_uuid' => $this->uuid])]);
|
||||
$mail->view('emails.s3-connection-error', ['name' => $this->name, 'reason' => $exception->getMessage(), 'url' => route('storage.show', ['storage_uuid' => $this->uuid])]);
|
||||
|
||||
// Load the team with its members and their roles explicitly
|
||||
$team = $this->team()->with(['members' => function ($query) {
|
||||
|
|
@ -183,11 +192,25 @@ public function testConnection(bool $shouldSave = false)
|
|||
$this->unusable_email_sent = true;
|
||||
}
|
||||
|
||||
throw $e;
|
||||
throw $exception;
|
||||
} finally {
|
||||
if ($shouldSave) {
|
||||
$this->save();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private function toUserFriendlyConnectionException(\Throwable $exception): \Throwable
|
||||
{
|
||||
$message = str($exception->getMessage())->lower();
|
||||
|
||||
if ($message->contains(['timed out', 'timeout', 'connection refused', 'could not resolve', 'curl error 28'])) {
|
||||
return new \RuntimeException(
|
||||
'Could not connect to the S3 endpoint within 15 seconds. Please verify the endpoint, bucket, credentials, region, and network/firewall settings.',
|
||||
previous: $exception,
|
||||
);
|
||||
}
|
||||
|
||||
return $exception;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,6 +1,10 @@
|
|||
<?php
|
||||
|
||||
use App\Models\S3Storage;
|
||||
use Illuminate\Support\Facades\Storage;
|
||||
use Tests\TestCase;
|
||||
|
||||
uses(TestCase::class);
|
||||
|
||||
test('S3Storage model has correct cast definitions', function () {
|
||||
$s3Storage = new S3Storage;
|
||||
|
|
@ -45,9 +49,72 @@
|
|||
expect($s3Storage->awsUrl())->toBe('https://minio.example.com:9000/backups');
|
||||
});
|
||||
|
||||
test('S3Storage model is guarded correctly', function () {
|
||||
test('S3Storage model fillable attributes are configured correctly', function () {
|
||||
$s3Storage = new S3Storage;
|
||||
|
||||
// The model should have $guarded = [] which means everything is fillable
|
||||
expect($s3Storage->getGuarded())->toBe([]);
|
||||
expect($s3Storage->getFillable())->toBe([
|
||||
'name',
|
||||
'description',
|
||||
'region',
|
||||
'key',
|
||||
'secret',
|
||||
'bucket',
|
||||
'endpoint',
|
||||
'is_usable',
|
||||
'unusable_email_sent',
|
||||
]);
|
||||
});
|
||||
|
||||
test('S3Storage connection validation uses short s3 client timeouts', function () {
|
||||
$disk = Mockery::mock();
|
||||
$disk->expects('files')->once()->andReturn([]);
|
||||
|
||||
Storage::expects('build')
|
||||
->once()
|
||||
->with(Mockery::on(function (array $config) {
|
||||
expect($config['http']['connect_timeout'])->toBe(15);
|
||||
expect($config['http']['timeout'])->toBe(15);
|
||||
|
||||
return true;
|
||||
}))
|
||||
->andReturn($disk);
|
||||
|
||||
$s3Storage = new S3Storage;
|
||||
$s3Storage->setRawAttributes([
|
||||
'name' => 'Test S3',
|
||||
'region' => 'us-east-1',
|
||||
'key' => null,
|
||||
'secret' => null,
|
||||
'bucket' => 'test-bucket',
|
||||
'endpoint' => 'https://s3.amazonaws.com',
|
||||
]);
|
||||
|
||||
$s3Storage->testConnection();
|
||||
|
||||
expect($s3Storage->is_usable)->toBeTrue();
|
||||
});
|
||||
|
||||
test('S3Storage connection validation returns friendly timeout error', function () {
|
||||
$disk = Mockery::mock();
|
||||
$disk->expects('files')
|
||||
->once()
|
||||
->andThrow(new RuntimeException('cURL error 28: Operation timed out after 15000 milliseconds'));
|
||||
|
||||
Storage::expects('build')->once()->andReturn($disk);
|
||||
|
||||
$s3Storage = new S3Storage;
|
||||
$s3Storage->setRawAttributes([
|
||||
'name' => 'Test S3',
|
||||
'region' => 'us-east-1',
|
||||
'key' => null,
|
||||
'secret' => null,
|
||||
'bucket' => 'test-bucket',
|
||||
'endpoint' => 'https://s3.amazonaws.com',
|
||||
'unusable_email_sent' => true,
|
||||
]);
|
||||
|
||||
expect(fn () => $s3Storage->testConnection())
|
||||
->toThrow(RuntimeException::class, 'Could not connect to the S3 endpoint within 15 seconds.');
|
||||
|
||||
expect($s3Storage->is_usable)->toBeFalse();
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue