fix(virus-scanner): rework error logging in pino (#7128)

Pino has in-built serialisers for logging Errors, but only on Errors or Errors in nested `err` properties. Change logging accordingly.
opengovsg · Mar 8, 2024 · d68b84a · d68b84a
1 parent da0d5b7
commit d68b84a
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 22 deletions.
diff --git a/serverless/virus-scanner/src/__tests/index.spec.ts b/serverless/virus-scanner/src/__tests/index.spec.ts
@@ -97,7 +97,7 @@ describe('handler', () => {
     expect(mockLoggerWarn).toHaveBeenCalledWith(
       expect.objectContaining({
         message: 'File not found',
-        error: new Error('File not found'),
+        err: new Error('File not found'),
         quarantineFileKey: mockUUID,
       }),
     )
@@ -175,7 +175,7 @@ describe('handler', () => {
     expect(mockLoggerError).toHaveBeenCalledWith(
       expect.objectContaining({
         message: 'Failed to scan file',
-        error: new Error('Failed to scan file'),
+        err: new Error('Failed to scan file'),
         quarantineFileKey: mockUUID,
       }),
     )
@@ -248,7 +248,7 @@ describe('handler', () => {
     expect(mockLoggerError).toHaveBeenCalledWith(
       expect.objectContaining({
         message: 'Failed to move file to clean bucket',
-        error: new Error('Failed to move file'),
+        err: new Error('Failed to move file'),
         bucket: 'local-virus-scanner-quarantine-bucket',
         key: mockUUID,
       }),

diff --git a/serverless/virus-scanner/src/__tests/s3.service.spec.ts b/serverless/virus-scanner/src/__tests/s3.service.spec.ts
@@ -145,7 +145,7 @@ describe('S3Service', () => {
         expect.objectContaining({
           bucketName: 'bucketName',
           objectKey: 'objectKey',
-          error: new Error('Body is empty'),
+          err: new Error('Body is empty'),
         }),
         'Failed to get object from s3',
       )
@@ -172,7 +172,7 @@ describe('S3Service', () => {
         expect.objectContaining({
           bucketName: 'bucketName',
           objectKey: 'objectKey',
-          error: new Error('VersionId is empty'),
+          err: new Error('VersionId is empty'),
         }),
         'Failed to get object from s3',
       )

diff --git a/serverless/virus-scanner/src/index.ts b/serverless/virus-scanner/src/index.ts
@@ -66,10 +66,10 @@ export const handler = async (
       bucketName: quarantineBucket,
       objectKey: quarantineFileKey,
     })
-  } catch (error) {
+  } catch (err) {
     logger.warn({
       message: 'File not found',
-      error,
+      err,
       quarantineFileKey,
     })
     return {
@@ -88,10 +88,10 @@ export const handler = async (
   let scanResult
   try {
     scanResult = await scanFileStream(body)
-  } catch (error) {
+  } catch (err) {
     logger.error({
       message: 'Failed to scan file',
-      error,
+      err,
       quarantineFileKey,
     })
     return {
@@ -122,11 +122,11 @@ export const handler = async (
         objectKey: quarantineFileKey,
         versionId,
       })
-    } catch (error) {
+    } catch (err) {
       // Log but do not halt execution as we still want to return 400 for malicious file
       logger.error({
         message: 'Failed to delete file from quarantine bucket',
-        error,
+        err,
         key: quarantineFileKey,
       })
     }
@@ -158,10 +158,10 @@ export const handler = async (
         destinationBucketName: cleanBucket,
         destinationObjectKey: cleanFileKey,
       })
-    } catch (error) {
+    } catch (err) {
       logger.error({
         message: 'Failed to move file to clean bucket',
-        error,
+        err,
         bucket: quarantineBucket,
         key: quarantineFileKey,
       })

diff --git a/serverless/virus-scanner/src/s3.service.ts b/serverless/virus-scanner/src/s3.service.ts
@@ -72,17 +72,17 @@ export class S3Service {
       )
 
       return { body, versionId } as GetS3FileStreamResult
-    } catch (error) {
+    } catch (err) {
       this.logger.error(
         {
           bucketName,
           objectKey,
-          error,
+          err,
         },
         'Failed to get object from s3',
       )
 
-      throw error
+      throw err
     }
   }
 
@@ -112,17 +112,17 @@ export class S3Service {
         },
         'Deleted document from s3',
       )
-    } catch (error) {
+    } catch (err) {
       this.logger.error(
         {
           bucketName,
           objectKey,
-          error,
+          err,
         },
         'Failed to delete object from s3',
       )
 
-      throw error
+      throw err
     }
   }
 
@@ -189,20 +189,20 @@ export class S3Service {
       )
 
       return VersionId
-    } catch (error) {
+    } catch (err) {
       this.logger.error(
         {
           sourceBucketName,
           sourceObjectKey,
           sourceObjectVersionId,
           destinationBucketName,
           destinationObjectKey,
-          error,
+          err,
         },
         'Failed to move object in s3',
       )
 
-      throw error
+      throw err
     }
   }
 }