From 985d0e7986b2cad23bb08ae0561b2a6ff9afdf9e Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= Date: Sun, 2 Jul 2023 17:13:05 +0200 Subject: [PATCH] fix: ensure task error proper throw with worker-threads MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Signed-off-by: Jérôme Benoit --- src/pools/abstract-pool.ts | 4 +--- src/worker/abstract-worker.ts | 10 +++++----- src/worker/cluster-worker.ts | 5 ----- src/worker/thread-worker.ts | 5 +++++ tests/pools/cluster/fixed.test.js | 18 ++++++++++++------ tests/pools/thread/fixed.test.js | 20 ++++++++++++-------- 6 files changed, 35 insertions(+), 27 deletions(-) diff --git a/src/pools/abstract-pool.ts b/src/pools/abstract-pool.ts index f71bf200..52a2aa38 100644 --- a/src/pools/abstract-pool.ts +++ b/src/pools/abstract-pool.ts @@ -832,9 +832,7 @@ export abstract class AbstractPool< if (this.emitter != null) { this.emitter.emit(PoolEvents.taskError, message.taskError) } - promiseResponse.reject( - `${message.taskError.message} on worker '${message.taskError.workerId}'` - ) + promiseResponse.reject(message.taskError.message) } else { promiseResponse.resolve(message.data as Response) } diff --git a/src/worker/abstract-worker.ts b/src/worker/abstract-worker.ts index 63d14b2e..f291c524 100644 --- a/src/worker/abstract-worker.ts +++ b/src/worker/abstract-worker.ts @@ -209,7 +209,7 @@ export abstract class AbstractWorker< * @returns The error message. */ protected handleError (e: Error | string): string { - return e as string + return e instanceof Error ? e.message : e } /** @@ -233,11 +233,11 @@ export abstract class AbstractWorker< id: message.id }) } catch (e) { - const err = this.handleError(e as Error) + const errorMessage = this.handleError(e as Error | string) this.sendToMainWorker({ taskError: { workerId: this.id, - message: err, + message: errorMessage, data: message.data }, id: message.id @@ -270,11 +270,11 @@ export abstract class AbstractWorker< return null }) .catch(e => { - const err = this.handleError(e as Error) + const errorMessage = this.handleError(e as Error | string) this.sendToMainWorker({ taskError: { workerId: this.id, - message: err, + message: errorMessage, data: message.data }, id: message.id diff --git a/src/worker/cluster-worker.ts b/src/worker/cluster-worker.ts index 180a5273..c43e7f75 100644 --- a/src/worker/cluster-worker.ts +++ b/src/worker/cluster-worker.ts @@ -52,9 +52,4 @@ export class ClusterWorker< protected sendToMainWorker (message: MessageValue): void { this.getMainWorker().send(message) } - - /** @inheritDoc */ - protected handleError (e: Error | string): string { - return e instanceof Error ? e.message : e - } } diff --git a/src/worker/thread-worker.ts b/src/worker/thread-worker.ts index 2b3df583..09135aff 100644 --- a/src/worker/thread-worker.ts +++ b/src/worker/thread-worker.ts @@ -56,4 +56,9 @@ export class ThreadWorker< protected sendToMainWorker (message: MessageValue): void { this.getMainWorker().postMessage(message) } + + /** @inheritDoc */ + protected handleError (e: Error | string): string { + return e as string + } } diff --git a/tests/pools/cluster/fixed.test.js b/tests/pools/cluster/fixed.test.js index 60dac401..ccdd2870 100644 --- a/tests/pools/cluster/fixed.test.js +++ b/tests/pools/cluster/fixed.test.js @@ -145,8 +145,12 @@ describe('Fixed cluster pool test suite', () => { } expect(inError).toBeDefined() expect(typeof inError === 'string').toBe(true) - expect(inError).toContain('Error Message from ClusterWorker on worker') - expect(taskError.data).toStrictEqual(data) + expect(inError).toBe('Error Message from ClusterWorker') + expect(taskError).toStrictEqual({ + workerId: expect.any(Number), + message: 'Error Message from ClusterWorker', + data + }) expect( errorPool.workerNodes.some( workerNode => workerNode.usage.tasks.failed === 1 @@ -168,10 +172,12 @@ describe('Fixed cluster pool test suite', () => { } expect(inError).toBeDefined() expect(typeof inError === 'string').toBe(true) - expect(inError).toContain( - 'Error Message from ClusterWorker:async on worker' - ) - expect(taskError.data).toStrictEqual(data) + expect(inError).toBe('Error Message from ClusterWorker:async') + expect(taskError).toStrictEqual({ + workerId: expect.any(Number), + message: 'Error Message from ClusterWorker:async', + data + }) expect( asyncErrorPool.workerNodes.some( workerNode => workerNode.usage.tasks.failed === 1 diff --git a/tests/pools/thread/fixed.test.js b/tests/pools/thread/fixed.test.js index f0448953..d0e31bab 100644 --- a/tests/pools/thread/fixed.test.js +++ b/tests/pools/thread/fixed.test.js @@ -147,10 +147,12 @@ describe('Fixed thread pool test suite', () => { expect(inError).toBeInstanceOf(Error) expect(inError.message).toBeDefined() expect(typeof inError.message === 'string').toBe(true) - expect(inError.message).toContain( - 'Error Message from ThreadWorker on worker' - ) - expect(taskError.data).toStrictEqual(data) + expect(inError.message).toBe('Error Message from ThreadWorker') + expect(taskError).toStrictEqual({ + workerId: expect.any(Number), + message: new Error('Error Message from ThreadWorker'), + data + }) expect( errorPool.workerNodes.some( workerNode => workerNode.usage.tasks.failed === 1 @@ -174,10 +176,12 @@ describe('Fixed thread pool test suite', () => { expect(inError).toBeInstanceOf(Error) expect(inError.message).toBeDefined() expect(typeof inError.message === 'string').toBe(true) - expect(inError.message).toContain( - 'Error Message from ThreadWorker:async on worker' - ) - expect(taskError.data).toStrictEqual(data) + expect(inError.message).toBe('Error Message from ThreadWorker:async') + expect(taskError).toStrictEqual({ + workerId: expect.any(Number), + message: new Error('Error Message from ThreadWorker:async'), + data + }) expect( asyncErrorPool.workerNodes.some( workerNode => workerNode.usage.tasks.failed === 1 -- 2.34.1