From 310de0aa5d5b3887a4de4ef6a0b42e7b7a560987 Mon Sep 17 00:00:00 2001
From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= <jerome.benoit@sap.com>
Date: Wed, 16 Aug 2023 16:52:33 +0200
Subject: [PATCH] refactor: cleanup message passing code
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

Signed-off-by: Jérôme Benoit <jerome.benoit@sap.com>
---
 src/pools/abstract-pool.ts    |  4 ++-
 src/worker/abstract-worker.ts | 55 ++++++++++++++++++-----------------
 2 files changed, 32 insertions(+), 27 deletions(-)

diff --git a/src/pools/abstract-pool.ts b/src/pools/abstract-pool.ts
index 06e69a6e..18d4fb83 100644
--- a/src/pools/abstract-pool.ts
+++ b/src/pools/abstract-pool.ts
@@ -510,7 +510,9 @@ export abstract class AbstractPool<
    * @throws {@link https://nodejs.org/api/errors.html#class-error} If the worker id is invalid.
    */
   private checkMessageWorkerId (message: MessageValue<Response>): void {
-    if (
+    if (message.workerId == null) {
+      throw new Error('Worker message received without worker id')
+    } else if (
       message.workerId != null &&
       this.getWorkerNodeKeyByWorkerId(message.workerId) === -1
     ) {
diff --git a/src/worker/abstract-worker.ts b/src/worker/abstract-worker.ts
index ff31fc7c..950738b2 100644
--- a/src/worker/abstract-worker.ts
+++ b/src/worker/abstract-worker.ts
@@ -320,24 +320,25 @@ export abstract class AbstractWorker<
   protected messageListener (message: MessageValue<Data>): void {
     if (this.isMain) {
       throw new Error('Cannot handle message to worker in main worker')
+    } else if (message.workerId == null) {
+      throw new Error('Message worker id is not set')
     } else if (message.workerId != null && message.workerId !== this.id) {
       throw new Error(
         `Message worker id ${message.workerId} does not match the worker id ${this.id}`
       )
-    } else if (message.workerId === this.id) {
-      if (message.statistics != null) {
-        // Statistics message received
-        this.statistics = message.statistics
-      } else if (message.checkActive != null) {
-        // Check active message received
-        message.checkActive ? this.startCheckActive() : this.stopCheckActive()
-      } else if (message.taskId != null && message.data != null) {
-        // Task message received
-        this.run(message)
-      } else if (message.kill === true) {
-        // Kill message received
-        this.handleKillMessage(message)
-      }
+    }
+    if (message.statistics != null) {
+      // Statistics message received
+      this.statistics = message.statistics
+    } else if (message.checkActive != null) {
+      // Check active message received
+      message.checkActive ? this.startCheckActive() : this.stopCheckActive()
+    } else if (message.taskId != null && message.data != null) {
+      // Task message received
+      this.run(message)
+    } else if (message.kill === true) {
+      // Kill message received
+      this.handleKillMessage(message)
     }
   }
 
@@ -473,26 +474,27 @@ export abstract class AbstractWorker<
     fn: TaskSyncFunction<Data, Response>,
     task: Task<Data>
   ): void {
+    const { name, taskId, data } = task
     try {
-      let taskPerformance = this.beginTaskPerformance(task.name)
-      const res = fn(task.data)
+      let taskPerformance = this.beginTaskPerformance(name)
+      const res = fn(data)
       taskPerformance = this.endTaskPerformance(taskPerformance)
       this.sendToMainWorker({
         data: res,
         taskPerformance,
         workerId: this.id,
-        taskId: task.taskId
+        taskId
       })
     } catch (e) {
       const errorMessage = this.handleError(e as Error | string)
       this.sendToMainWorker({
         taskError: {
-          name: task.name ?? DEFAULT_TASK_NAME,
+          name: name ?? DEFAULT_TASK_NAME,
           message: errorMessage,
-          data: task.data
+          data
         },
         workerId: this.id,
-        taskId: task.taskId
+        taskId
       })
     } finally {
       this.updateLastTaskTimestamp()
@@ -509,15 +511,16 @@ export abstract class AbstractWorker<
     fn: TaskAsyncFunction<Data, Response>,
     task: Task<Data>
   ): void {
-    let taskPerformance = this.beginTaskPerformance(task.name)
-    fn(task.data)
+    const { name, taskId, data } = task
+    let taskPerformance = this.beginTaskPerformance(name)
+    fn(data)
       .then((res) => {
         taskPerformance = this.endTaskPerformance(taskPerformance)
         this.sendToMainWorker({
           data: res,
           taskPerformance,
           workerId: this.id,
-          taskId: task.taskId
+          taskId
         })
         return null
       })
@@ -525,12 +528,12 @@ export abstract class AbstractWorker<
         const errorMessage = this.handleError(e as Error | string)
         this.sendToMainWorker({
           taskError: {
-            name: task.name ?? DEFAULT_TASK_NAME,
+            name: name ?? DEFAULT_TASK_NAME,
             message: errorMessage,
-            data: task.data
+            data
           },
           workerId: this.id,
-          taskId: task.taskId
+          taskId
         })
       })
       .finally(() => {
-- 
2.34.1