fix(s3): improve logging, add test coverage

src/__tests__/s3.service.spec.ts

@@ -15,15 +15,13 @@ let getResult = {
 
 type Result = typeof getResult
 
+const mockS3Send = jest.fn<unknown, [Result, ...unknown[]], Result>()
+
 jest.mock('@aws-sdk/client-s3', () => {
   return {
     S3Client: jest.fn().mockImplementation(() => {
       return {
-        send: jest
-          .fn<unknown, [Result, ...unknown[]], Result>()
-          .mockImplementation((result) => {
-            return result
-          }),
+        send: mockS3Send,
       }
     }),
     CopyObjectCommand: jest.fn().mockImplementation(() => {
@@ -59,6 +57,10 @@ const testConfig = {
 describe('S3Service', () => {
   beforeEach(() => {
     jest.clearAllMocks()
+    mockS3Send.mockReset()
+    mockS3Send.mockImplementation((result) => {
+      return result
+    })
   })
   describe('moveS3File', () => {
     it('should move file to clean bucket and log', async () => {
@@ -175,6 +177,23 @@ describe('S3Service', () => {
         'Moved document in s3',
       )
     })
+
+    it('should throw on bad versionId from S3 client', async () => {
+      // Arrange
+      mockS3Send.mockReturnValue({ VersionId: undefined })
+      const mockS3Service = new S3Service(testConfig, mockLogger)
+
+      // Act and Assert
+      await expect(
+        mockS3Service.moveS3File({
+          sourceBucketName: 'sourceBucketName',
+          sourceObjectKey: 'sourceObjectKey',
+          sourceObjectVersionId: 'sourceObjectVersionId',
+          destinationBucketName: 'destinationBucketName',
+          destinationObjectKey: 'destinationObjectKey',
+        }),
+      ).rejects.toThrow(new Error('VersionId is empty'))
+    })
   })
   describe('getS3FileStreamWithVersionId', () => {
     it('should return file stream with version id', async () => {
@@ -345,5 +364,33 @@ describe('S3Service', () => {
         'Deleted document from s3',
       )
     })
+
+    it('should log and rethrow on error while deleting', async () => {
+      // Arrange
+      const error = new Error('Failed to delete')
+      mockS3Send.mockRejectedValue(error as never)
+      const mockS3Service = new S3Service(testConfig, mockLogger)
+
+      // Act
+      await expect(
+        mockS3Service.deleteS3File({
+          bucketName: 'bucketName',
+          objectKey: 'objectKey',
+          versionId: 'mockVersion',
+        }),
+      ).rejects.toThrow(error)
+
+      // Assert
+      expect(DeleteObjectCommand).toHaveBeenCalledTimes(1)
+      expect(DeleteObjectCommand).toHaveBeenCalledWith({
+        Key: 'objectKey',
+        Bucket: 'bucketName',
+        VersionId: 'mockVersion',
+      })
+      expect(mockLoggerError).toHaveBeenCalledWith(
+        error,
+        'Failed to delete object objectKey from s3 bucket bucketName',
+      )
+    })
   })
 })

src/s3.service.ts

@@ -123,12 +123,8 @@ export class S3Service {
       )
     } catch (error) {
       this.logger.error(
-        {
-          bucketName,
-          objectKey,
-          error,
-        },
-        'Failed to delete object from s3',
+        error,
+        `Failed to delete object ${objectKey} from s3 bucket ${bucketName}`,
       )
 
       throw error