Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
95 commits
Select commit Hold shift + click to select a range
4a04d9f
gcp client code migration from aws-sdk v2 to v3
benzekrimaha Sep 18, 2025
e977b6b
code migration from aws-sdk v2 to v3
benzekrimaha Sep 18, 2025
f2d1862
tests update post migration from aws-sdk v2 to v3
benzekrimaha Sep 18, 2025
dfa82ee
fixup post reviews
benzekrimaha Oct 6, 2025
6caa9ee
Bump @eslint/plugin-kit from 0.3.5 to 0.4.0
dependabot[bot] Sep 30, 2025
b0d7e19
fixup post reviews
benzekrimaha Oct 13, 2025
30a9a72
post reviews fixups
benzekrimaha Oct 15, 2025
b1ab841
fixup thanks to cloudserver failure
benzekrimaha Oct 29, 2025
2725f3e
fixup double middleware usage
benzekrimaha Oct 30, 2025
5082db4
fixup double middleware usage
benzekrimaha Oct 30, 2025
9050d1d
fixupp
benzekrimaha Oct 30, 2025
537edef
fixupp
benzekrimaha Oct 30, 2025
f42ae42
fixupp
benzekrimaha Oct 30, 2025
afa98e4
fixupp
benzekrimaha Oct 30, 2025
bbc5736
fixupp
benzekrimaha Oct 30, 2025
1860908
fixupp
benzekrimaha Oct 30, 2025
c1e4353
wip
benzekrimaha Oct 31, 2025
f22d161
wip
benzekrimaha Oct 31, 2025
80a5bc1
wip
benzekrimaha Oct 31, 2025
15a219d
wip
benzekrimaha Oct 31, 2025
3628f54
wip
benzekrimaha Nov 3, 2025
ccbf1cb
region default value
benzekrimaha Nov 3, 2025
cb1406c
wip
benzekrimaha Nov 3, 2025
13566df
wip
benzekrimaha Nov 3, 2025
bc68e48
wip
benzekrimaha Nov 3, 2025
9b89515
wip
benzekrimaha Nov 3, 2025
a62b0c3
wip
benzekrimaha Nov 4, 2025
c1cb299
wip
benzekrimaha Nov 4, 2025
80c71ab
wip
benzekrimaha Nov 4, 2025
e001134
wip
benzekrimaha Nov 4, 2025
63b5ed7
wip
benzekrimaha Nov 4, 2025
c1dbc0b
wip
benzekrimaha Nov 4, 2025
ad89172
wip
benzekrimaha Nov 4, 2025
f2ebcbe
wip
benzekrimaha Nov 4, 2025
1f1282b
wip
benzekrimaha Nov 4, 2025
f2cc101
wip
benzekrimaha Nov 4, 2025
391b5f9
wip
benzekrimaha Nov 4, 2025
5888102
wip
benzekrimaha Nov 4, 2025
f9fa2b5
wip
williamlardier Nov 4, 2025
dab2b55
wip
benzekrimaha Nov 5, 2025
09a4d72
wip
benzekrimaha Nov 5, 2025
3bc9cda
wip
benzekrimaha Nov 5, 2025
eea48d8
wip
benzekrimaha Nov 5, 2025
ae16e73
wip
benzekrimaha Nov 5, 2025
b638159
wip
benzekrimaha Nov 5, 2025
e82f62b
wip
benzekrimaha Nov 5, 2025
beddd18
removing console.log
benzekrimaha Nov 5, 2025
f44d462
removing console.log
benzekrimaha Nov 5, 2025
41e613f
removing console.log
benzekrimaha Nov 5, 2025
0267c8a
adding next marker to listing to avoid infinite loop
benzekrimaha Nov 6, 2025
f2ed4e0
delete object tagging debug logs
benzekrimaha Nov 6, 2025
1836433
delete object tagging debug logs
benzekrimaha Nov 6, 2025
0f74c7d
delete object tagging debug logs
benzekrimaha Nov 6, 2025
9c44beb
delete object tagging debug logs
benzekrimaha Nov 6, 2025
e6b3c80
delete object tagging debug logs
benzekrimaha Nov 6, 2025
7b42293
get object fixup
benzekrimaha Nov 6, 2025
448991a
get object fixup
benzekrimaha Nov 6, 2025
8dfa1f1
get object fixup
benzekrimaha Nov 6, 2025
89c858f
get object fixup
benzekrimaha Nov 6, 2025
28666f9
get object fixup
benzekrimaha Nov 6, 2025
e0289d8
Handle undefined or array headers in canonicalHeaderList
williamlardier Nov 7, 2025
5c2e854
Default the LocationConstraint if GetBucketLocation doesn't return
williamlardier Nov 7, 2025
ce54892
testing
williamlardier Nov 7, 2025
32b466b
Stringify metadata headers in SDK requests
williamlardier Nov 7, 2025
fc47a03
[to be removed] debug logs on mpu
benzekrimaha Nov 12, 2025
5773f10
fix on mpu
benzekrimaha Nov 12, 2025
92209c8
[to be removed] debug logs on mpu
benzekrimaha Nov 12, 2025
ea0d556
[to be removed] debug logs on mpu
benzekrimaha Nov 12, 2025
6cd93bd
[to be removed] debug logs on mpu
benzekrimaha Nov 12, 2025
cd8c386
[to be removed] debug logs on mpu
benzekrimaha Nov 13, 2025
15155a9
[to be removed] debug logs on mpu
benzekrimaha Nov 13, 2025
ef70715
[to be removed] debug logs on mpu
benzekrimaha Nov 13, 2025
49ed63f
[to be removed] debug logs on mpu
benzekrimaha Nov 13, 2025
4e6ef09
[to be removed] debug logs on mpu
benzekrimaha Nov 13, 2025
5860483
[to be removed] debug logs on mpu
benzekrimaha Nov 13, 2025
c38a472
[to be removed] debug logs on mpu
benzekrimaha Nov 13, 2025
6d9293f
[to be removed] debug logs on mpu
benzekrimaha Nov 13, 2025
a219f29
[to be removed] debug logs on mpu
benzekrimaha Nov 13, 2025
86c9aa6
[to be removed] debug logs on mpu
benzekrimaha Nov 13, 2025
af97f95
[to be removed] debug logs on mpu
benzekrimaha Nov 13, 2025
454e8de
[to be removed] debug logs on mpu
benzekrimaha Nov 13, 2025
634e3d6
[to be removed] debug logs on mpu
benzekrimaha Nov 13, 2025
faa6574
[to be removed] debug logs on mpu
benzekrimaha Nov 13, 2025
cf48ed1
[to be removed] debug logs on mpu
benzekrimaha Nov 13, 2025
b6fc714
[to be removed] debug logs on mpu
benzekrimaha Nov 17, 2025
848973d
[to be removed] debug logs on mpu
benzekrimaha Nov 17, 2025
2679b84
[to be removed] debug logs on mpu
benzekrimaha Nov 17, 2025
4d1537c
fixup etag undefined
benzekrimaha Nov 18, 2025
11de95d
fixup etag undefined
benzekrimaha Nov 18, 2025
d6fb784
fix unit tests
benzekrimaha Nov 18, 2025
8a74d92
readable stream fixup
benzekrimaha Nov 19, 2025
54c1219
readable stream fixup
benzekrimaha Nov 19, 2025
030f658
readable stream fixup
benzekrimaha Nov 19, 2025
aff2e55
readable stream fixup
benzekrimaha Nov 19, 2025
97d7f9a
readable stream fixup
benzekrimaha Nov 19, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,8 @@ export const storage = {
AwsClient: require('./lib/storage/data/external/AwsClient'),
AzureClient: require('./lib/storage/data/external/AzureClient'),
GcpClient: require('./lib/storage/data/external/GcpClient'),
GCP: require('./lib/storage/data/external/GCP/GcpService'),
GCP: require('./lib/storage/data/external/GCP'),
GcpUtils: require('./lib/storage/data/external/GCP/GcpUtils'),
GcpSigner: require('./lib/storage/data/external/GCP/GcpSigner'),
PfsClient: require('./lib/storage/data/external/PfsClient'),
backendUtils: require('./lib/storage/data/external/utils'),
},
Expand Down
11 changes: 10 additions & 1 deletion lib/auth/v2/getCanonicalizedAmzHeaders.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,16 @@ export default function getCanonicalizedAmzHeaders(headers: Record<string, strin
(val: string) => val.startsWith('x-amz-');
const amzHeaders = Object.keys(headers)
.filter(filterFn)
.map(val => [val.trim(), headers[val].trim()]);
.map(val => {
const headerValue = headers[val];
// AWS SDK v3 can pass header values as arrays (for multiple values),
// strings, or other types. We need to normalize them before calling .trim()
// Per HTTP spec and AWS Signature v2, multiple values are joined with commas
const stringValue = Array.isArray(headerValue)
? headerValue.join(',')
: String(headerValue);
return [val.trim(), stringValue.trim()];
});
/*
AWS docs state that duplicate headers should be combined
in the same header with values concatenated with
Expand Down
9 changes: 8 additions & 1 deletion lib/auth/v4/createCanonicalRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,14 @@ export default function createCanonicalRequest(
// canonical headers
const canonicalHeadersList = signedHeadersList.map((signedHeader: any) => {
if (pHeaders[signedHeader] !== undefined) {
const trimmedHeader = pHeaders[signedHeader]
const headerValue = pHeaders[signedHeader];
// AWS SDK v3 can pass header values as arrays (for multiple values),
// strings, or other types. We need to normalize them before calling .trim()
// Per HTTP spec and AWS Signature v4, multiple values are joined with commas
const stringValue = Array.isArray(headerValue)
? headerValue.join(',')
: String(headerValue);
const trimmedHeader = stringValue
.trim().replace(/\s+/g, ' ');
return `${signedHeader}:${trimmedHeader}\n`;
}
Expand Down
4 changes: 1 addition & 3 deletions lib/auth/v4/streamingV4/constructChunkStringToSign.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,7 @@ export default function constructChunkStringToSign(
currentChunkHash = constants.emptyStringHash;
} else {
const hash = crypto.createHash('sha256');
const temp = justDataChunk instanceof Buffer
? hash.update(justDataChunk)
: hash.update(justDataChunk, 'binary');
const temp = hash.update(justDataChunk);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As justDataChunk is either a string or a buffer w emust keep the 'binary' option in the case it is a string

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const temp = hash.update(justDataChunk);
const temp = justDataChunk instanceof Buffer
? hash.update(justDataChunk)
: hash.update(justDataChunk, 'binary');

If we are sure this is a buffer, we should update the function from

    justDataChunk?: Buffer | string,

to

    justDataChunk?: Buffer,

currentChunkHash = temp.digest('hex');
}
return `AWS4-HMAC-SHA256-PAYLOAD\n${timestamp}\n` +
Expand Down
169 changes: 66 additions & 103 deletions lib/network/kmsAWS/Client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,16 @@
import { arsenalErrorAWSKMS } from '../utils';
import { Agent as HttpAgent } from 'http';
import { Agent as HttpsAgent } from 'https';
import { KMS, AWSError } from 'aws-sdk';
import { KMSClient,
CreateKeyCommand,
ScheduleKeyDeletionCommand,
GenerateDataKeyCommand,
EncryptCommand,
DecryptCommand,
ListKeysCommand,
NotFoundException,
KMSInvalidStateException } from '@aws-sdk/client-kms';
const { NodeHttpHandler } = require('@smithy/node-http-handler');
import * as werelogs from 'werelogs';
import assert from 'assert';
import { KMSInterface, KmsBackend, getKeyIdFromArn, KmsProtocol, KmsType, makeBackend } from '../KMSInterface';
Expand Down Expand Up @@ -45,42 +54,39 @@ interface ClientOptions {

export default class Client implements KMSInterface {
private _supportsDefaultKeyPerAccount: boolean;
private client: KMS;
private client: KMSClient;
public readonly backend: KmsBackend<KmsType.external>;
public readonly noAwsArn?: boolean;

constructor(options: ClientOptions) {
this._supportsDefaultKeyPerAccount = true;
const { providerName, tls, ak, sk, region, endpoint, noAwsArn } = options.kmsAWS;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lint : nit

const { providerName,tls, ak, sk, region, endpoint, noAwsArn } = options.kmsAWS;

const httpOptions = tls ? {
agent: new HttpsAgent({
const requestHandler = new NodeHttpHandler({
httpAgent: !tls ? new HttpAgent({
keepAlive: true,
}) : undefined,
httpsAgent: tls ? new HttpsAgent({
keepAlive: true,
rejectUnauthorized: tls.rejectUnauthorized,
ca: tls.ca,
cert: tls.cert,
minVersion: tls.minVersion,
maxVersion: tls.maxVersion,
key: tls.key,
}),
} : {
agent: new HttpAgent({
keepAlive: true,
}),
};
}) : undefined,
});

const credentials = (ak && sk) ? {
credentials: {
accessKeyId: ak,
secretAccessKey: sk,
},
accessKeyId: ak,
secretAccessKey: sk,
} : undefined;

this.client = new KMS({
this.client = new KMSClient({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know how we can re-introduce the keepAlive here? Is it from the lib directly?

region,
endpoint,
httpOptions,
...credentials,
credentials,
requestHandler,
});
this.backend = makeBackend(KmsType.external, KmsProtocol.aws_kms, providerName);
this.noAwsArn = noAwsArn;
Expand Down Expand Up @@ -118,31 +124,22 @@ export default class Client implements KMSInterface {

createMasterKey(logger: werelogs.Logger, cb: (err: Error | null, keyId?: string, keyArn?: string) => void): void {
logger.debug('AWS KMS: creating master encryption key');
this.client.createKey({}, (err: AWSError, data) => {
if (err) {
const error = arsenalErrorAWSKMS(err);
logger.error('AWS KMS: failed to create master encryption key', { err });
cb(error);
return;
}
this.client.send(new CreateKeyCommand({})).then(data => {
const keyMetadata = data?.KeyMetadata;
logger.debug("AWS KMS: master encryption key created", { KeyMetadata: keyMetadata });
let keyId: string;
if (this.noAwsArn) {
// Use KeyId when ARN is not wanted
// eslint-disable-next-line @typescript-eslint/no-non-null-asserted-optional-chain
keyId = keyMetadata?.KeyId!;
keyId = keyMetadata?.KeyId || '';
} else {
// Prefer ARN, but fall back to KeyId if ARN is missing
// eslint-disable-next-line @typescript-eslint/no-non-null-asserted-optional-chain
keyId = keyMetadata?.Arn ?? keyMetadata?.KeyId!;
keyId = keyMetadata?.Arn ?? (keyMetadata?.KeyId || '');
Copy link
Contributor

@williamlardier williamlardier Sep 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is important top keep the comment about double arn prefix just below.

This one:

            // May produce double arn prefix: scality arn + aws arn
            // arn:scality:kms:external:aws_kms:custom:key/arn:aws:kms:region:accountId:key/cbd69d33-ba8e-4b56-8cfe
            // If this is a problem, a config flag should be used to hide the scality arn when returning the KMS KeyId
            // or aws arn when creating the KMS Key

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(sill applicable)

}
// May produce double arn prefix: scality arn + aws arn
// arn:scality:kms:external:aws_kms:custom:key/arn:aws:kms:region:accountId:key/cbd69d33-ba8e-4b56-8cfe
// If this is a problem, a config flag should be used to hide the scality arn when returning the KMS KeyId
// or aws arn when creating the KMS Key
const arn = `${this.backend.arnPrefix}${keyId}`;
cb(null, keyId, arn);
}).catch(err => {
const error = arsenalErrorAWSKMS(err);
logger.error('AWS KMS: failed to create master encryption key', { err });
cb(error);
});
}

Expand All @@ -163,30 +160,24 @@ export default class Client implements KMSInterface {
KeyId: masterKeyId,
PendingWindowInDays: 7,
};
this.client.scheduleKeyDeletion(params, (err: AWSError, data) => {
if (err) {
if (err.code === 'NotFoundException' || err.code === 'KMSInvalidStateException') {
// master key does not exist or is already pending deletion
logger.info('AWS KMS: key does not exist or is already pending deletion',
{ masterKeyId, error: err });
cb(null);
return;
}

const error = arsenalErrorAWSKMS(err);
logger.error('AWS KMS: failed to delete master encryption key', { err });
cb(error);
return;
}
const command = new ScheduleKeyDeletionCommand(params);

this.client.send(command).then(data => {
if (data?.KeyState && data.KeyState !== 'PendingDeletion') {
const error = arsenalErrorAWSKMS('key is not in PendingDeletion state');
logger.error('AWS KMS: failed to delete master encryption key', { err, data });
logger.error('AWS KMS: failed to delete master encryption key', { data });
cb(error);
return;
}

cb(null);
}).catch(err => {
if (err instanceof NotFoundException || err instanceof KMSInvalidStateException) {
logger.info('AWS KMS: key does not exist or is already pending deletion', { masterKeyId, error: err });
return cb(null);
Comment on lines +175 to +176
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we return no error we want at least to have a warning log I think?

Suggested change
logger.info('AWS KMS: key does not exist or is already pending deletion', { masterKeyId, error: err });
return cb(null);
logger.warn('AWS KMS: key does not exist or is already pending deletion', { masterKeyId, error: err });
return cb(null);

}
const error = arsenalErrorAWSKMS(err);
logger.error('AWS KMS: failed to delete master encryption key', { err });
return cb(error);
});
}

Expand All @@ -199,31 +190,24 @@ export default class Client implements KMSInterface {
const masterKeyId = getKeyIdFromArn(masterKeyIdOrArn);
logger.debug("AWS KMS: generating data key", { cryptoScheme, masterKeyId, masterKeyIdOrArn });
assert.strictEqual(cryptoScheme, 1);

const params = {
KeyId: masterKeyId,
KeySpec: 'AES_256',
KeySpec: 'AES_256' as const,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
KeySpec: 'AES_256' as const,
KeySpec: 'AES_256',

};

this.client.generateDataKey(params, (err: AWSError, data) => {
if (err) {
const error = arsenalErrorAWSKMS(err);
logger.error('AWS KMS: failed to generate data key', { err });
cb(error);
return;
}

this.client.send(new GenerateDataKeyCommand(params)).then(data => {
if (!data) {
const error = arsenalErrorAWSKMS("failed to generate data key: empty response");
logger.error("AWS KMS: failed to generate data key: empty response");
cb(error);
return;
}

const isolatedPlaintext = this.safePlaintext(data.Plaintext as Buffer);

logger.debug('AWS KMS: data key generated');
cb(null, isolatedPlaintext, Buffer.from(data.CiphertextBlob as Uint8Array));
}).catch(err => {
const error = arsenalErrorAWSKMS(err);
logger.error('AWS KMS: failed to generate data key', { err });
cb(error);
});
}

Expand All @@ -244,14 +228,7 @@ export default class Client implements KMSInterface {
Plaintext: plainTextDataKey,
};

this.client.encrypt(params, (err: AWSError, data) => {
if (err) {
const error = arsenalErrorAWSKMS(err);
logger.error('AWS KMS: failed to cipher data key', { err });
cb(error);
return;
}

this.client.send(new EncryptCommand(params)).then(data => {
if (!data) {
const error = arsenalErrorAWSKMS("failed to cipher data key: empty response");
logger.error("AWS KMS: failed to cipher data key: empty response");
Expand All @@ -262,6 +239,10 @@ export default class Client implements KMSInterface {
logger.debug('AWS KMS: data key ciphered');
cb(null, Buffer.from(data.CiphertextBlob as Uint8Array));
return;
}).catch(err => {
const error = arsenalErrorAWSKMS(err);
logger.error('AWS KMS: failed to cipher data key', { err });
cb(error);
});
}

Expand All @@ -281,14 +262,7 @@ export default class Client implements KMSInterface {
CiphertextBlob: cipheredDataKey,
};

this.client.decrypt(params, (err: AWSError, data) => {
if (err) {
const error = arsenalErrorAWSKMS(err);
logger.error('AWS KMS: failed to decipher data key', { err });
cb(error);
return;
}

this.client.send(new DecryptCommand(params)).then(data => {
if (!data) {
const error = arsenalErrorAWSKMS("failed to decipher data key: empty response");
logger.error("AWS KMS: failed to decipher data key: empty response");
Expand All @@ -300,41 +274,30 @@ export default class Client implements KMSInterface {

logger.debug('AWS KMS: data key deciphered');
cb(null, isolatedPlaintext);
}).catch(err => {
const error = arsenalErrorAWSKMS(err);
logger.error('AWS KMS: failed to decipher data key', { err });
cb(error);
});
}

/**
* NOTE1: S3C-4833 KMS healthcheck is disabled in CloudServer
* NOTE2: The best approach for implementing the AWS KMS health check is still under consideration.
* In the meantime, this method is commented out to prevent potential issues related to costs or permissions.
*
* Reasons for commenting out:
* - frequent API calls can lead to increased expenses.
* - access key secret key used must have `kms:ListKeys` permissions
*
* Future potential actions:
* - implement caching mechanisms to reduce the number of API calls.
* - differentiate between error types (e.g., 500 vs. 403) for more effective error handling.
* Healthcheck function to verify KMS connectivity
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the old comment mentioning

 * NOTE2: The best approach for implementing the AWS KMS health check is still under consideration.

Maybe you can check with Nicolas if the new implementation is OK, as now this is enabled?

*/
/*
healthcheck(logger: werelogs.Logger, cb: (err: Error | null) => void): void {
logger.debug("AWS KMS: performing healthcheck");

const params = {
const command = new ListKeysCommand({
Limit: 1,
};

this.client.listKeys(params, (err, data) => {
if (err) {
const error = arsenalErrorAWSKMS(err);
logger.error("AWS KMS healthcheck: failed to list keys", { err });
cb(error);
return;
}
});

this.client.send(command).then(() => {
logger.debug("AWS KMS healthcheck: list keys succeeded");
cb(null);
}).catch(err => {
const error = arsenalErrorAWSKMS(err);
logger.error("AWS KMS healthcheck: failed to list keys", { err });
cb(error);
});
}
*/
}
28 changes: 22 additions & 6 deletions lib/network/utils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import type { AWSError } from 'aws-sdk';
import { ArsenalError, errorInstances } from '../errors';
import { allowedKmsErrors } from '../errors/kmsErrors';

Expand Down Expand Up @@ -32,20 +31,37 @@ export function arsenalErrorKMIP(err: string | Error) {

const allowedKmsErrorCodes = Object.keys(allowedKmsErrors) as unknown as (keyof typeof allowedKmsErrors)[];

// Local AWSError type for compatibility with v3 error handling
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure to understand the goal of the comment here, can you explain 🙏

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

before we used to rely on the old aws sdk library which is no longer the case with the migration so had to define the type in order to keep the compatibility with the error handling

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then maybe just a naming like ArsenalAWSError ?

Copy link
Contributor Author

@benzekrimaha benzekrimaha Oct 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keeping the same name avoids additionnal not needed changes as the PR is already extensive

export type AWSError = Error & {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure you can remove this and use the S3ServiceException type, this is exposed by the @aws-sdk/client-s3 module and all exceptions related to S3 are extending this base type, it has all the fields you have here, with the advantage of being typed & documented

name?: string;
$fault?: 'client' | 'server';
$metadata?: {
httpStatusCode?: number;
requestId?: string;
attempts?: number;
totalRetryDelay?: number;
};
$retryable?: {
throttling?: boolean;
};
message?: string;
};

function isAWSError(err: string | Error | AWSError): err is AWSError {
return (err as AWSError).code !== undefined
&& (err as AWSError).retryable !== undefined;
return (err as AWSError).name !== undefined
&& (err as AWSError).$metadata !== undefined;
}

export function arsenalErrorAWSKMS(err: string | Error | AWSError) {
if (isAWSError(err)) {
if (allowedKmsErrorCodes.includes(err.code as keyof typeof allowedKmsErrors)) {
return errorInstances[`KMS.${err.code}`].customizeDescription(err.message);
const errorCode = err.name;
if (allowedKmsErrorCodes.includes(errorCode as keyof typeof allowedKmsErrors)) {
return errorInstances[`KMS.${errorCode}`].customizeDescription(err.message);
} else {
// Encapsulate into a generic ArsenalError but keep the aws error code
return ArsenalError.unflatten({
is_arsenal_error: true,
type: `KMS.${err.code}`, // aws s3 prefix kms errors with KMS.
type: `KMS.${errorCode}`, // aws s3 prefix kms errors with KMS.
code: 500,
description: `unexpected AWS_KMS error`,
stack: err.stack,
Expand Down
Loading
Loading