mirror of
https://github.com/n8n-io/n8n.git
synced 2026-06-03 18:27:09 +02:00
fix(core): Add configurable retries and error details to S3 (backport to 1.x) (#29810)
Co-authored-by: Iván Ovejero <ivov.src@gmail.com>
This commit is contained in:
parent
c7cb00a77d
commit
83023cce28
|
|
@ -45,6 +45,8 @@ describe('ObjectStoreService', () => {
|
|||
authAutoDetect: false,
|
||||
},
|
||||
protocol: 'https',
|
||||
forcePathStyle: true,
|
||||
maxAttempts: 3,
|
||||
});
|
||||
|
||||
let objectStoreService: ObjectStoreService;
|
||||
|
|
@ -74,6 +76,22 @@ describe('ObjectStoreService', () => {
|
|||
forcePathStyle: true,
|
||||
region: mockBucket.region,
|
||||
credentials,
|
||||
maxAttempts: 3,
|
||||
});
|
||||
});
|
||||
|
||||
it('should return client config with forcePathStyle disabled when configured', () => {
|
||||
s3Config.host = 'example.com';
|
||||
s3Config.forcePathStyle = false;
|
||||
|
||||
const clientConfig = objectStoreService.getClientConfig();
|
||||
|
||||
expect(clientConfig).toEqual({
|
||||
endpoint: 'https://example.com',
|
||||
forcePathStyle: false,
|
||||
region: mockBucket.region,
|
||||
credentials,
|
||||
maxAttempts: 3,
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -85,6 +103,7 @@ describe('ObjectStoreService', () => {
|
|||
expect(clientConfig).toEqual({
|
||||
region: mockBucket.region,
|
||||
credentials,
|
||||
maxAttempts: 3,
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -95,6 +114,7 @@ describe('ObjectStoreService', () => {
|
|||
|
||||
expect(clientConfig).toEqual({
|
||||
region: mockBucket.region,
|
||||
maxAttempts: 3,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -47,6 +47,14 @@ export class ObjectStoreConfig {
|
|||
@Env('N8N_EXTERNAL_STORAGE_S3_PROTOCOL', protocolSchema)
|
||||
protocol: Protocol = 'https';
|
||||
|
||||
/** Whether to use path-style addressing for S3 requests (e.g. `https://host/bucket` instead of `https://bucket.host`) */
|
||||
@Env('N8N_EXTERNAL_STORAGE_S3_FORCE_PATH_STYLE')
|
||||
forcePathStyle: boolean = true;
|
||||
|
||||
/** Maximum number of retry attempts for S3 requests */
|
||||
@Env('N8N_EXTERNAL_STORAGE_S3_MAX_ATTEMPTS')
|
||||
maxAttempts: number = 3;
|
||||
|
||||
@Nested
|
||||
bucket: ObjectStoreBucketConfig = {} as ObjectStoreBucketConfig;
|
||||
|
||||
|
|
|
|||
|
|
@ -16,7 +16,7 @@ import {
|
|||
} from '@aws-sdk/client-s3';
|
||||
import { Logger } from '@n8n/backend-common';
|
||||
import { Service } from '@n8n/di';
|
||||
import { UnexpectedError } from 'n8n-workflow';
|
||||
import { ensureError, UnexpectedError } from 'n8n-workflow';
|
||||
import { createHash } from 'node:crypto';
|
||||
import { Readable } from 'node:stream';
|
||||
|
||||
|
|
@ -50,12 +50,12 @@ export class ObjectStoreService {
|
|||
|
||||
/** This generates the config for the S3Client to make it work in all various auth configurations */
|
||||
getClientConfig() {
|
||||
const { host, bucket, protocol, credentials } = this.s3Config;
|
||||
const { host, bucket, protocol, credentials, maxAttempts } = this.s3Config;
|
||||
const clientConfig: S3ClientConfig = {};
|
||||
const endpoint = host ? `${protocol}://${host}` : undefined;
|
||||
if (endpoint) {
|
||||
clientConfig.endpoint = endpoint;
|
||||
clientConfig.forcePathStyle = true; // Needed for non-AWS S3 compatible services
|
||||
clientConfig.forcePathStyle = this.s3Config.forcePathStyle;
|
||||
}
|
||||
if (bucket.region.length) {
|
||||
clientConfig.region = bucket.region;
|
||||
|
|
@ -66,6 +66,7 @@ export class ObjectStoreService {
|
|||
secretAccessKey: credentials.accessSecret,
|
||||
};
|
||||
}
|
||||
clientConfig.maxAttempts = maxAttempts;
|
||||
return clientConfig;
|
||||
}
|
||||
|
||||
|
|
@ -89,7 +90,7 @@ export class ObjectStoreService {
|
|||
const command = new HeadBucketCommand({ Bucket: this.bucket });
|
||||
await this.s3Client.send(command);
|
||||
} catch (e) {
|
||||
throw new UnexpectedError('Request to S3 failed', { cause: e });
|
||||
this.handleS3Error(e);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -118,7 +119,7 @@ export class ObjectStoreService {
|
|||
const command = new PutObjectCommand(params);
|
||||
return await this.s3Client.send(command);
|
||||
} catch (e) {
|
||||
throw new UnexpectedError('Request to S3 failed', { cause: e });
|
||||
this.handleS3Error(e);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -146,7 +147,8 @@ export class ObjectStoreService {
|
|||
|
||||
return await streamToBuffer(body as Readable);
|
||||
} catch (e) {
|
||||
throw new UnexpectedError('Request to S3 failed', { cause: e });
|
||||
if (e instanceof UnexpectedError) throw e;
|
||||
this.handleS3Error(e);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -181,7 +183,7 @@ export class ObjectStoreService {
|
|||
|
||||
return headers;
|
||||
} catch (e) {
|
||||
throw new UnexpectedError('Request to S3 failed', { cause: e });
|
||||
this.handleS3Error(e);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -198,7 +200,7 @@ export class ObjectStoreService {
|
|||
this.logger.debug('Sending DELETE request to S3', { bucket: this.bucket, key: fileId });
|
||||
return await this.s3Client.send(command);
|
||||
} catch (e) {
|
||||
throw new UnexpectedError('Request to S3 failed', { cause: e });
|
||||
this.handleS3Error(e);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -226,7 +228,7 @@ export class ObjectStoreService {
|
|||
const command = new DeleteObjectsCommand(params);
|
||||
return await this.s3Client.send(command);
|
||||
} catch (e) {
|
||||
throw new UnexpectedError('Request to S3 failed', { cause: e });
|
||||
this.handleS3Error(e);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -252,7 +254,7 @@ export class ObjectStoreService {
|
|||
|
||||
return items;
|
||||
} catch (e) {
|
||||
throw new UnexpectedError('Request to S3 failed', { cause: e });
|
||||
this.handleS3Error(e);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -290,7 +292,12 @@ export class ObjectStoreService {
|
|||
nextContinuationToken: response.NextContinuationToken,
|
||||
};
|
||||
} catch (e) {
|
||||
throw new UnexpectedError('Request to S3 failed', { cause: e });
|
||||
this.handleS3Error(e);
|
||||
}
|
||||
}
|
||||
|
||||
private handleS3Error(e: unknown): never {
|
||||
const error = ensureError(e);
|
||||
throw new UnexpectedError(`Request to S3 failed: ${error.message}`, { cause: error });
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user