From 848335461ea32f19f33b48cf1aa464204560da14 Mon Sep 17 00:00:00 2001 From: Vidhan Shah Date: Mon, 24 Nov 2025 16:42:38 +0530 Subject: [PATCH] PR comments Signed-off-by: Vidhan Shah --- .../src/stages/publish/awsS3.test.ts | 73 ++++++------------- .../techdocs-node/src/stages/publish/awsS3.ts | 4 +- 2 files changed, 24 insertions(+), 53 deletions(-) diff --git a/plugins/techdocs-node/src/stages/publish/awsS3.test.ts b/plugins/techdocs-node/src/stages/publish/awsS3.test.ts index e33c5c2612..8be8c05994 100644 --- a/plugins/techdocs-node/src/stages/publish/awsS3.test.ts +++ b/plugins/techdocs-node/src/stages/publish/awsS3.test.ts @@ -305,7 +305,7 @@ describe('AwsS3Publish', () => { describe('retry mechanism', () => { it('should retry with custom retry strategy', async () => { const publisher = await createPublisherFromConfig(); - const customRetryStrategy = jest.fn((error: any) => { + const customRetryStrategy = jest.fn(error => { return error.name === 'NetworkingError'; }); @@ -320,11 +320,10 @@ describe('AwsS3Publish', () => { ) .resolvesOnce({ Contents: [] }); - await (publisher as any).retryOperation( + await (publisher as AwsS3Publish).retryOperation( async () => { - return (s3Mock.send as any)( - new ListObjectsV2Command({ Bucket: 'bucketName' }), - ); + const command = new ListObjectsV2Command({ Bucket: 'bucketName' }); + return (publisher as AwsS3Publish).storageClient.send(command); }, 'TestOperation', 3, @@ -347,11 +346,10 @@ describe('AwsS3Publish', () => { ) .resolvesOnce({ Contents: [] }); - await (publisher as any).retryOperation( + await (publisher as AwsS3Publish).retryOperation( async () => { - return (s3Mock.send as any)( - new ListObjectsV2Command({ Bucket: 'bucketName' }), - ); + const command = new ListObjectsV2Command({ Bucket: 'bucketName' }); + return (publisher as AwsS3Publish).storageClient.send(command); }, 'TestOperation', 3, @@ -371,11 +369,10 @@ describe('AwsS3Publish', () => { ) .resolvesOnce({ Contents: [] }); - await (publisher as any).retryOperation( + await (publisher as AwsS3Publish).retryOperation( async () => { - return (s3Mock.send as any)( - new ListObjectsV2Command({ Bucket: 'bucketName' }), - ); + const command = new ListObjectsV2Command({ Bucket: 'bucketName' }); + return (publisher as AwsS3Publish).storageClient.send(command); }, 'TestOperation', 3, @@ -395,40 +392,16 @@ describe('AwsS3Publish', () => { ) .resolvesOnce({ Contents: [] }); - await (publisher as any).retryOperation( + await (publisher as AwsS3Publish).retryOperation( async () => { - return (s3Mock.send as any)( - new ListObjectsV2Command({ Bucket: 'bucketName' }), - ); + const command = new ListObjectsV2Command({ Bucket: 'bucketName' }); + return (publisher as AwsS3Publish).storageClient.send(command); }, 'TestOperation', 3, ); }); - it('should not retry on non-retriable 4xx errors', async () => { - const publisher = await createPublisherFromConfig(); - s3Mock.on(ListObjectsV2Command).rejectsOnce( - new S3ServiceException({ - name: 'BadRequest', - $fault: 'client', - $metadata: { httpStatusCode: 400 }, - }), - ); - - await expect( - (publisher as any).retryOperation( - async () => { - return (s3Mock.send as any)( - new ListObjectsV2Command({ Bucket: 'bucketName' }), - ); - }, - 'TestOperation', - 3, - ), - ).rejects.toHaveProperty('name', 'BadRequest'); - }); - it('should use exact error code matching for transient errors', async () => { const publisher = await createPublisherFromConfig(); // Test that ConnectionError (exact match) is retried, but ConnectionErrorSomething (substring) is not @@ -443,11 +416,10 @@ describe('AwsS3Publish', () => { ) .resolvesOnce({ Contents: [] }); - await (publisher as any).retryOperation( + await (publisher as AwsS3Publish).retryOperation( async () => { - return (s3Mock.send as any)( - new ListObjectsV2Command({ Bucket: 'bucketName' }), - ); + const command = new ListObjectsV2Command({ Bucket: 'bucketName' }); + return (publisher as AwsS3Publish).storageClient.send(command); }, 'TestOperation', 3, @@ -469,14 +441,13 @@ describe('AwsS3Publish', () => { ) .resolvesOnce({ Contents: [] }); - await (publisher as any).retryOperation( + await (publisher as AwsS3Publish).retryOperation( async () => { - return (s3Mock.send as any)( - new ListObjectsV2Command({ Bucket: 'bucketName' }), - ); + const command = new ListObjectsV2Command({ Bucket: 'bucketName' }); + return (publisher as AwsS3Publish).storageClient.send(command); }, 'TestOperation', - 2, + 3, ); const elapsedTime = Date.now() - startTime; @@ -743,7 +714,7 @@ describe('AwsS3Publish', () => { }); it('should return an error if the techdocs_metadata.json file cannot be read from stream', async () => { - s3Mock.on(GetObjectCommand).callsFake((_: any) => { + s3Mock.on(GetObjectCommand).callsFake(() => { return { Body: new ErrorReadable('No stream!'), }; @@ -882,7 +853,7 @@ describe('AwsS3Publish', () => { }); it('should return 404 if file cannot be read from stream', async () => { - s3Mock.on(GetObjectCommand).callsFake((_: any) => { + s3Mock.on(GetObjectCommand).callsFake(() => { return { Body: new ErrorReadable('No stream!'), }; diff --git a/plugins/techdocs-node/src/stages/publish/awsS3.ts b/plugins/techdocs-node/src/stages/publish/awsS3.ts index 0e84a433f6..157c2ffdeb 100644 --- a/plugins/techdocs-node/src/stages/publish/awsS3.ts +++ b/plugins/techdocs-node/src/stages/publish/awsS3.ts @@ -84,7 +84,7 @@ const streamToBuffer = (stream: Readable): Promise => { }; export class AwsS3Publish implements PublisherBase { - private readonly storageClient: S3Client; + public readonly storageClient: S3Client; private readonly bucketName: string; private readonly legacyPathCasing: boolean; private readonly logger: LoggerService; @@ -268,7 +268,7 @@ export class AwsS3Publish implements PublisherBase { /** * Custom retry wrapper for S3 operations with detailed error handling. */ - private async retryOperation( + public async retryOperation( operation: () => Promise, operationName: string, maxAttempts: number = 3,