From f87e3fe029e48d8964722da762326e531c2256ee Mon Sep 17 00:00:00 2001
From: Rogger Valverde <rogger.valverde@uni.pe>
Date: Wed, 15 May 2024 23:23:58 -0600
Subject: [PATCH] fix(job): validate job existence when adding a log (#2562)

---
 src/classes/job.ts        | 20 +++-----------------
 src/classes/scripts.ts    | 29 ++++++++++++++++++++++++++++-
 src/commands/addLog-2.lua | 30 ++++++++++++++++++++++++++++++
 tests/test_job.ts         | 10 ++++++++++
 4 files changed, 71 insertions(+), 18 deletions(-)
 create mode 100644 src/commands/addLog-2.lua

diff --git a/src/classes/job.ts b/src/classes/job.ts
index 978671c222..4e9dd3a75c 100644
--- a/src/classes/job.ts
+++ b/src/classes/job.ts
@@ -409,29 +409,15 @@ export class Job<
    *
    * @returns The total number of log entries for this job so far.
    */
-  static async addJobLog(
+  static addJobLog(
     queue: MinimalQueue,
     jobId: string,
     logRow: string,
     keepLogs?: number,
   ): Promise<number> {
-    const client = await queue.client;
-    const logsKey = queue.toKey(jobId) + ':logs';
-
-    const multi = client.multi();
-
-    multi.rpush(logsKey, logRow);
-
-    if (keepLogs) {
-      multi.ltrim(logsKey, -keepLogs, -1);
-    }
-
-    const result = (await multi.exec()) as [
-      [Error, number],
-      [Error, string] | undefined,
-    ];
+    const scripts = (queue as any).scripts as Scripts;
 
-    return keepLogs ? Math.min(keepLogs, result[0][1]) : result[0][1];
+    return scripts.addLog(jobId, logRow, keepLogs);
   }
 
   toJSON() {
diff --git a/src/classes/scripts.ts b/src/classes/scripts.ts
index 4a1c35e4d8..f8a029d744 100644
--- a/src/classes/scripts.ts
+++ b/src/classes/scripts.ts
@@ -326,7 +326,7 @@ export class Scripts {
     }
   }
 
-  async updateProgress<T = any, R = any, N extends string = string>(
+  async updateProgress(
     jobId: string,
     progress: number | object,
   ): Promise<void> {
@@ -352,6 +352,33 @@ export class Scripts {
     }
   }
 
+  async addLog(
+    jobId: string,
+    logRow: string,
+    keepLogs?: number,
+  ): Promise<number> {
+    const client = await this.queue.client;
+
+    const keys: (string | number)[] = [
+      this.queue.toKey(jobId),
+      this.queue.toKey(jobId) + ':logs',
+    ];
+
+    const result = await (<any>client).addLog(
+      keys.concat([jobId, logRow, keepLogs ? keepLogs : '']),
+    );
+
+    if (result < 0) {
+      throw this.finishedErrors({
+        code: result,
+        jobId,
+        command: 'addLog',
+      });
+    }
+
+    return result;
+  }
+
   protected moveToFinishedArgs<T = any, R = any, N extends string = string>(
     job: MinimalJob<T, R, N>,
     val: any,
diff --git a/src/commands/addLog-2.lua b/src/commands/addLog-2.lua
new file mode 100644
index 0000000000..92748de9a9
--- /dev/null
+++ b/src/commands/addLog-2.lua
@@ -0,0 +1,30 @@
+--[[
+  Add job log
+
+  Input:
+    KEYS[1] job id key
+    KEYS[2] job logs key
+
+    ARGV[1] id
+    ARGV[2] log
+    ARGV[3] keepLogs
+
+  Output:
+    -1 - Missing job.
+]]
+local rcall = redis.call
+
+if rcall("EXISTS", KEYS[1]) == 1 then -- // Make sure job exists
+  local logCount = rcall("RPUSH", KEYS[2], ARGV[2])
+
+  if ARGV[3] ~= '' then
+    local keepLogs = tonumber(ARGV[3])
+    rcall("LTRIM", KEYS[2], -keepLogs, -1)
+
+    return math.min(keepLogs, logCount)
+  end
+
+  return logCount
+else
+  return -1
+end
diff --git a/tests/test_job.ts b/tests/test_job.ts
index d0b0675d6c..97e4908d25 100644
--- a/tests/test_job.ts
+++ b/tests/test_job.ts
@@ -477,6 +477,16 @@ describe('Job', function () {
 
       expect(logs).to.be.eql({ logs: [firstLog, secondLog], count: 2 });
     });
+
+    describe('when job is removed', () => {
+      it('throws error', async function () {
+        const job = await Job.create(queue, 'test', { foo: 'bar' });
+        await job.remove();
+        await expect(job.log('oneLog')).to.be.rejectedWith(
+          `Missing key for job ${job.id}. addLog`,
+        );
+      });
+    });
   });
 
   describe('.clearLogs', () => {