#3565 - Virus Scanning - Retry on error (#3651)

- Ensured that a scanned file returning something different than true or
false will be retried later.
- Investigate on what is returned during a virus scan failed and check
if there is difference between null and undefined in the response and a
way to capture the error thrown.
- Created interface `VirusScanCode` to ensure expected behavior from
method `scanFile`.
- Added key `isServerAvailable` to the response for queue method
`performVirusScan`.
- Updated the virus-scan queue configuration as below (suggested 12
attempts every 30min).
```json
{
  "retry": 12,
  "cleanUpPeriod": 604800000,
  "retryInterval": 900000,
  "dashboardReadonly": false
}
```
- Removed the NotFound exception as it is not related to HTTP.
  - Made sure that error is thrown.
- Added E2E Test(s) to ensure that exception is thrown when the scanning
process does not complete.

![image](https://github.com/user-attachments/assets/87e6c425-b1da-4d40-bf34-182d259f8420)

**Step Not in AC** (Discussion)
- Set `virus_scan_status` to `VirusScanStatus.Pending` for error
handling and null response and include `VirusScanStatus.Pending` in the
where clause for getting a student file.

Screenshot of log messages for server unavailable during file scanning
in queue-consumers

![image](https://github.com/user-attachments/assets/266f2c6e-9f06-4f8d-875c-f1edf7eb9cde)

Screenshot of log messages in bull dashboard

![image](https://github.com/user-attachments/assets/4d544e65-08b8-457f-910b-e34623eed735)

**Rollback Evidence**

![image](https://github.com/user-attachments/assets/5f8c38a4-8fdc-49e4-817d-72e0fe73c25a)

sources/docker-compose.yml

@@ -235,6 +235,8 @@ services:
       interval: 5s
       timeout: 5s
       retries: 5
+    networks:
+      - local-network
   # - ClamAV
   # Web
   web:

sources/packages/backend/apps/db-migrations/src/migrations/1724108928803-UpdateVirusScanQueueConfiguration.ts

@@ -0,0 +1,21 @@
+import { MigrationInterface, QueryRunner } from "typeorm";
+import { getSQLFileData } from "../utilities/sqlLoader";
+
+export class UpdateVirusScanQueueConfiguration1724108928803
+  implements MigrationInterface
+{
+  public async up(queryRunner: QueryRunner): Promise<void> {
+    await queryRunner.query(
+      getSQLFileData("Update-virus-scan-queue-configuration.sql", "Queue"),
+    );
+  }
+
+  public async down(queryRunner: QueryRunner): Promise<void> {
+    await queryRunner.query(
+      getSQLFileData(
+        "Rollback-update-virus-scan-queue-configuration.sql",
+        "Queue",
+      ),
+    );
+  }
+}

sources/packages/backend/apps/db-migrations/src/sql/Queue/Rollback-update-virus-scan-queue-configuration.sql

@@ -0,0 +1,11 @@
+UPDATE
+    sims.queue_configurations
+SET
+    queue_configuration = '{
+        "retry": 3,
+        "retryInterval": 180000,
+        "dashboardReadonly": false,
+        "cleanUpPeriod": 604800000
+    }' :: json
+WHERE
+    queue_name = 'virus-scan';

sources/packages/backend/apps/db-migrations/src/sql/Queue/Update-virus-scan-queue-configuration.sql

@@ -0,0 +1,11 @@
+UPDATE
+    sims.queue_configurations
+SET
+    queue_configuration = '{
+        "retry": 12,
+        "retryInterval": 900000,
+        "dashboardReadonly": false,
+        "cleanUpPeriod": 604800000
+    }' :: json
+WHERE
+    queue_name = 'virus-scan';

sources/packages/backend/apps/queue-consumers/src/constants/error-code.constants.ts

@@ -1,3 +1,7 @@
-// Error code used when file virus scanning failed for some reason.
+// Error codes used when file virus scanning failed for some reason.
 // One possible reason could be the ClamAV server being down.
-export const UNABLE_TO_SCAN_FILE = "UNABLE_TO_SCAN_FILE";
+export const CONNECTION_FAILED = "CONNECTION_FAILED";
+export const FILE_NOT_FOUND = "FILE_NOT_FOUND";
+export const FILE_SCANNING_FAILED = "FILE_SCANNING_FAILED";
+export const SERVER_UNAVAILABLE = "SERVER_UNAVAILABLE";
+export const UNKNOWN_ERROR = "UNKNOWN_ERROR";

sources/packages/backend/apps/queue-consumers/src/processors/virus-scan/_tests_/virus-scan.processor.e2e-spec.ts

@@ -10,7 +10,7 @@ import {
   createFakeStudentFileUpload,
   E2EDataSources,
 } from "@sims/test-utils";
-import { ClamAVService } from "@sims/services";
+import { ClamAVError, ClamAVService } from "@sims/services";
 import { VirusScanQueueInDTO } from "@sims/services/queue";
 import { VirusScanProcessor } from "../virus-scan.processor";
 import { VirusScanStatus } from "@sims/sims-db";
@@ -40,6 +40,158 @@ describe(describeProcessorRootTest(QueueNames.FileVirusScanProcessor), () => {
     jest.resetAllMocks();
   });
 
+  it("Should throw an error when the student file is not found during virus scanning.", async () => {
+    // Arrange
+    const studentFile = createFakeStudentFileUpload();
+    studentFile.virusScanStatus = VirusScanStatus.InProgress;
+    await db.studentFile.save(studentFile);
+    const fakeUniqueFileName = "file.random";
+    const mockedJob = mockBullJob<VirusScanQueueInDTO>({
+      uniqueFileName: fakeUniqueFileName,
+      fileName: studentFile.fileName,
+    });
+
+    // Act
+    const errorMessage = `File ${fakeUniqueFileName} is not found or has already been scanned for viruses. Scanning the file for viruses is aborted.`;
+    await expect(
+      processor.performVirusScan(mockedJob.job),
+    ).rejects.toStrictEqual(new Error(errorMessage));
+    expect(
+      mockedJob.containLogMessages([
+        "Log details",
+        "Starting virus scan.",
+        errorMessage,
+      ]),
+    ).toBe(true);
+  });
+
+  it("Should throw an error when the connection to the virus scan server failed.", async () => {
+    // Arrange
+    const studentFile = createFakeStudentFileUpload();
+    studentFile.virusScanStatus = VirusScanStatus.InProgress;
+    await db.studentFile.save(studentFile);
+    clamAVServiceMock.scanFile = jest.fn(() => {
+      throw new ClamAVError("Connection refused", "ECONNREFUSED");
+    }); // Queued job.
+    const mockedJob = mockBullJob<VirusScanQueueInDTO>({
+      uniqueFileName: studentFile.uniqueFileName,
+      fileName: studentFile.fileName,
+    });
+
+    // Act
+    const errorMessage = `Unable to scan the file ${studentFile.uniqueFileName} for viruses. Connection to ClamAV server failed.`;
+    await expect(
+      processor.performVirusScan(mockedJob.job),
+    ).rejects.toStrictEqual(new Error(errorMessage));
+    expect(
+      mockedJob.containLogMessages([
+        "Log details",
+        "Starting virus scan.",
+        errorMessage,
+      ]),
+    ).toBe(true);
+    const scannedStudentFile = await db.studentFile.findOneBy({
+      id: studentFile.id,
+    });
+    expect(scannedStudentFile.virusScanStatus).toBe(VirusScanStatus.Pending);
+    expect(scannedStudentFile.fileName).toBe(studentFile.fileName);
+  });
+
+  it("Should throw an error when the virus scan server is unavailable.", async () => {
+    // Arrange
+    const studentFile = createFakeStudentFileUpload();
+    studentFile.virusScanStatus = VirusScanStatus.InProgress;
+    await db.studentFile.save(studentFile);
+    clamAVServiceMock.scanFile = jest.fn(() => {
+      throw new ClamAVError("Server not found", "ENOTFOUND");
+    }); // Queued job.
+    const mockedJob = mockBullJob<VirusScanQueueInDTO>({
+      uniqueFileName: studentFile.uniqueFileName,
+      fileName: studentFile.fileName,
+    });
+
+    // Act
+    const errorMessage = `Unable to scan the file ${studentFile.uniqueFileName} for viruses. ClamAV server is unavailable.`;
+    await expect(
+      processor.performVirusScan(mockedJob.job),
+    ).rejects.toStrictEqual(new Error(errorMessage));
+    expect(
+      mockedJob.containLogMessages([
+        "Log details",
+        "Starting virus scan.",
+        errorMessage,
+      ]),
+    ).toBe(true);
+    const scannedStudentFile = await db.studentFile.findOneBy({
+      id: studentFile.id,
+    });
+    expect(scannedStudentFile.virusScanStatus).toBe(VirusScanStatus.Pending);
+    expect(scannedStudentFile.fileName).toBe(studentFile.fileName);
+  });
+
+  it("Should throw an error when an unknown error occurred during virus scanning.", async () => {
+    // Arrange
+    const studentFile = createFakeStudentFileUpload();
+    studentFile.virusScanStatus = VirusScanStatus.InProgress;
+    await db.studentFile.save(studentFile);
+    clamAVServiceMock.scanFile = jest.fn(() => {
+      throw new ClamAVError("Unknown error", undefined);
+    }); // Queued job.
+    const mockedJob = mockBullJob<VirusScanQueueInDTO>({
+      uniqueFileName: studentFile.uniqueFileName,
+      fileName: studentFile.fileName,
+    });
+
+    // Act
+    const errorMessage = `Unable to scan the file ${studentFile.uniqueFileName} for viruses. Unknown error.`;
+    await expect(
+      processor.performVirusScan(mockedJob.job),
+    ).rejects.toStrictEqual(new Error(errorMessage));
+    expect(
+      mockedJob.containLogMessages([
+        "Log details",
+        "Starting virus scan.",
+        errorMessage,
+        errorMessage,
+      ]),
+    ).toBe(true);
+    const scannedStudentFile = await db.studentFile.findOneBy({
+      id: studentFile.id,
+    });
+    expect(scannedStudentFile.virusScanStatus).toBe(VirusScanStatus.Pending);
+    expect(scannedStudentFile.fileName).toBe(studentFile.fileName);
+  });
+
+  it("Should throw an error when file scanning failed during virus scanning.", async () => {
+    // Arrange
+    const studentFile = createFakeStudentFileUpload();
+    studentFile.virusScanStatus = VirusScanStatus.InProgress;
+    await db.studentFile.save(studentFile);
+    clamAVServiceMock.scanFile = jest.fn(() => Promise.resolve(null));
+    const mockedJob = mockBullJob<VirusScanQueueInDTO>({
+      uniqueFileName: studentFile.uniqueFileName,
+      fileName: studentFile.fileName,
+    });
+
+    // Act
+    const errorMessage = `Unable to scan the file ${studentFile.uniqueFileName} for viruses. File scanning failed due to unknown error.`;
+    await expect(
+      processor.performVirusScan(mockedJob.job),
+    ).rejects.toStrictEqual(new Error(errorMessage));
+    expect(
+      mockedJob.containLogMessages([
+        "Log details",
+        "Starting virus scan.",
+        errorMessage,
+      ]),
+    ).toBe(true);
+    const scannedStudentFile = await db.studentFile.findOneBy({
+      id: studentFile.id,
+    });
+    expect(scannedStudentFile.virusScanStatus).toBe(VirusScanStatus.Pending);
+    expect(scannedStudentFile.fileName).toBe(studentFile.fileName);
+  });
+
   it("Should update the file name and status when virus is detected.", async () => {
     // Arrange
     const studentFile = createFakeStudentFileUpload();

sources/packages/backend/apps/queue-consumers/src/processors/virus-scan/virus-scan.processor.ts

@@ -8,9 +8,8 @@ import {
   LoggerService,
   ProcessSummary,
 } from "@sims/utilities/logger";
-import { NotFoundException } from "@nestjs/common";
 import { logProcessSummaryToJobLogger } from "../../utilities";
-import { UNABLE_TO_SCAN_FILE } from "../../constants/error-code.constants";
+import { FILE_NOT_FOUND } from "../../constants/error-code.constants";
 
 @Processor(QueueNames.FileVirusScanProcessor)
 export class VirusScanProcessor {
@@ -26,7 +25,7 @@ export class VirusScanProcessor {
     job: Job<VirusScanQueueInDTO>,
   ): Promise<VirusScanResult> {
     const processSummary = new ProcessSummary();
-    let isInfected: boolean;
+    let isInfected: boolean | null = null;
     processSummary.info("Starting virus scan.");
     try {
       isInfected = await this.studentFileService.scanFile(
@@ -35,29 +34,27 @@ export class VirusScanProcessor {
       );
       processSummary.info("Completed virus scanning for the file.");
     } catch (error: unknown) {
-      if (error instanceof NotFoundException) {
-        // If the file is not present in the database,
-        // remove the file from the virus scan queue.
-        await job.discard();
-        processSummary.warn(
-          `File ${job.data.uniqueFileName} not found or has already been scanned for viruses. Scanning the file for viruses is aborted.`,
-        );
-      } else if (error instanceof CustomNamedError) {
-        if (error.name === UNABLE_TO_SCAN_FILE) {
-          processSummary.error(
-            `Unable to scan the file ${job.data.uniqueFileName} for viruses.`,
-          );
+      if (error instanceof CustomNamedError) {
+        const errorMessage = error.message;
+        if (error.name === FILE_NOT_FOUND) {
+          // If the file is not present in the database, remove the file from the virus scan queue.
+          await job.discard();
         }
-      } else {
-        processSummary.error(
-          `Error while scanning the file ${job.data.uniqueFileName} for viruses.`,
-        );
+        processSummary.error(errorMessage);
+        throw new Error(errorMessage, { cause: error });
       }
+      throw new Error(
+        "Unexpected error while executing the job, check logs for further details.",
+        { cause: error },
+      );
     } finally {
       this.logger.logProcessSummary(processSummary);
       await logProcessSummaryToJobLogger(processSummary, job);
     }
-    return { fileProcessed: job.data.fileName, isInfected };
+    return {
+      fileProcessed: job.data.fileName,
+      isInfected,
+    };
   }
 
   @InjectLogger()