Compare commits

...
Author SHA1 Message Date
Sonarly Claude Code da775d437e fix: add retry with credential refresh for S3 ExpiredToken errors
https://sonarly.com/issue/19873?type=bug

Workflow code actions fail when the S3 client's cached AWS temporary credentials expire, preventing the worker from reading built logic function code from S3.

Fix: Added retry-on-credential-error logic to `S3Driver.readFile()` in `s3.driver.ts`.

**What changed:**

1. **`s3.driver.ts`**: Added a constant `RETRYABLE_CREDENTIAL_ERROR_NAMES` listing AWS error names (`ExpiredToken`, `ExpiredTokenException`, `RequestExpired`) that indicate stale temporary credentials. Added a private `isRetryableCredentialError()` helper. In `readFile()`, when the initial S3 GetObject fails with one of these errors, the driver now logs a warning and retries once with a fresh command. The retry naturally causes the AWS SDK's `fromNodeProviderChain` credential provider to refresh the expired credentials.

2. **`s3.driver.spec.ts`**: Added three test cases for the new behavior:
   - Retry succeeds on `ExpiredToken` (verifies `send` is called twice)
   - `NoSuchKey` errors are not retried (still thrown as `FileStorageException`)
   - Non-retryable errors (e.g., `AccessDenied`) propagate immediately

The fix is minimal and surgical — only the `readFile` method (the affected code path from the stack trace) is modified. The retry is limited to exactly one attempt to avoid infinite loops. If the credential refresh itself fails, the error propagates naturally.
2026-03-31 07:40:20 +00:00
2 changed files with 109 additions and 1 deletions
@@ -1,14 +1,21 @@
import { Readable } from 'stream';
import { GetObjectCommand } from '@aws-sdk/client-s3';
import { getSignedUrl } from '@aws-sdk/s3-request-presigner';
import { S3Driver } from 'src/engine/core-modules/file-storage/drivers/s3.driver';
import { FileStorageExceptionCode } from 'src/engine/core-modules/file-storage/interfaces/file-storage-exception';
const mockSend = jest.fn();
jest.mock('@aws-sdk/client-s3', () => {
const actual = jest.requireActual('@aws-sdk/client-s3');
return {
...actual,
S3: jest.fn().mockImplementation(() => ({})),
S3: jest.fn().mockImplementation(() => ({
send: mockSend,
})),
};
});
@@ -16,6 +23,76 @@ jest.mock('@aws-sdk/s3-request-presigner', () => ({
getSignedUrl: jest.fn(),
}));
describe('S3Driver.readFile', () => {
afterEach(() => {
jest.clearAllMocks();
});
it('should retry once on ExpiredToken and succeed', async () => {
const expiredError = new Error('The provided token has expired.');
Object.defineProperty(expiredError, 'name', { value: 'ExpiredToken' });
const readableBody = Readable.from(Buffer.from('file-content'));
mockSend
.mockRejectedValueOnce(expiredError)
.mockResolvedValueOnce({ Body: readableBody });
const driver = new S3Driver({
bucketName: 'test-bucket',
region: 'us-east-1',
});
const result = await driver.readFile({ filePath: 'some/file.js' });
expect(result).toBeInstanceOf(Readable);
expect(mockSend).toHaveBeenCalledTimes(2);
});
it('should throw NoSuchKey as FileStorageException without retrying', async () => {
const noSuchKeyError = new Error('Not found');
Object.defineProperty(noSuchKeyError, 'name', { value: 'NoSuchKey' });
mockSend.mockRejectedValueOnce(noSuchKeyError);
const driver = new S3Driver({
bucketName: 'test-bucket',
region: 'us-east-1',
});
await expect(
driver.readFile({ filePath: 'missing/file.js' }),
).rejects.toMatchObject({
code: FileStorageExceptionCode.FILE_NOT_FOUND,
});
expect(mockSend).toHaveBeenCalledTimes(1);
});
it('should throw non-retryable errors immediately', async () => {
const accessDeniedError = new Error('Access denied');
Object.defineProperty(accessDeniedError, 'name', {
value: 'AccessDenied',
});
mockSend.mockRejectedValueOnce(accessDeniedError);
const driver = new S3Driver({
bucketName: 'test-bucket',
region: 'us-east-1',
});
await expect(
driver.readFile({ filePath: 'some/file.js' }),
).rejects.toThrow('Access denied');
expect(mockSend).toHaveBeenCalledTimes(1);
});
});
describe('S3Driver.getPresignedUrl', () => {
afterEach(() => {
jest.clearAllMocks();
@@ -37,6 +37,14 @@ export interface S3DriverOptions extends S3ClientConfig {
presignEndpoint?: string;
}
// AWS error names that indicate expired/invalid temporary credentials
// and are recoverable by retrying (which triggers credential refresh)
const RETRYABLE_CREDENTIAL_ERROR_NAMES = [
'ExpiredToken',
'ExpiredTokenException',
'RequestExpired',
];
export class S3Driver implements StorageDriver {
private s3Client: S3;
private presignClient: S3 | undefined;
@@ -71,6 +79,10 @@ export class S3Driver implements StorageDriver {
return this.s3Client;
}
private isRetryableCredentialError(error: { name?: string }): boolean {
return RETRYABLE_CREDENTIAL_ERROR_NAMES.includes(error.name ?? '');
}
async readFile(params: { filePath: string }): Promise<Readable> {
const command = new GetObjectCommand({
Key: params.filePath,
@@ -93,6 +105,25 @@ export class S3Driver implements StorageDriver {
);
}
if (this.isRetryableCredentialError(error)) {
this.logger.warn(
`S3 credential error (${error.name}), retrying once to trigger credential refresh`,
);
const retryCommand = new GetObjectCommand({
Key: params.filePath,
Bucket: this.bucketName,
});
const file = await this.s3Client.send(retryCommand);
if (!file || !file.Body || !(file.Body instanceof Readable)) {
throw new Error('Unable to get file stream');
}
return Readable.from(file.Body);
}
throw error;
}
}