From 3235b657c313e8e8a9e03e70a14c5f07157697c8 Mon Sep 17 00:00:00 2001 From: Josh Wulf Date: Wed, 31 May 2023 17:25:37 +1200 Subject: [PATCH] fixes #316 --- CHANGELOG.md | 8 ++ .../Worker-Failure-Retries.spec.ts | 110 ++++++++++++++++++ .../integration/Worker-Failure.spec.ts | 8 ++ .../testdata/Worker-Failure-Retries.bpmn | 55 +++++++++ src/lib/ZBWorkerBase.ts | 25 ++-- 5 files changed, 194 insertions(+), 12 deletions(-) create mode 100644 src/__tests__/integration/Worker-Failure-Retries.spec.ts create mode 100644 src/__tests__/testdata/Worker-Failure-Retries.bpmn diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e8f973..d792dc3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,11 @@ +# Version 8.2.3 + +## Fixes + +_Things that were broken and are now fixed._ + +- The object signature for `job.fail()` did not correctly apply an explicit value for `retries`. As a result, job retries would decrement automatically if this signature and option were used. The value is now correctly parsed and applied, and job retry count can be explicitly set in the `job.fail()` command with the object signature. Thanks to [@patozgg](https://github.com/patozgg) for reporting this. See [#316](https://github.com/camunda-community-hub/zeebe-client-node-js/issues/316) for more details. + # Version 8.2.2 ## Chores diff --git a/src/__tests__/integration/Worker-Failure-Retries.spec.ts b/src/__tests__/integration/Worker-Failure-Retries.spec.ts new file mode 100644 index 0000000..67c52a4 --- /dev/null +++ b/src/__tests__/integration/Worker-Failure-Retries.spec.ts @@ -0,0 +1,110 @@ +import { ZBClient } from '../..' +import { CreateProcessInstanceResponse } from '../../lib/interfaces-grpc-1.0' + +// const trace = (res: T) => { +// // tslint:disable-next-line: no-console +// console.log(res) +// return res +// } +process.env.ZEEBE_NODE_LOG_LEVEL = process.env.ZEEBE_NODE_LOG_LEVEL || 'NONE' +jest.setTimeout(60000) + +let zbc: ZBClient +let wf: CreateProcessInstanceResponse | undefined + +beforeEach(() => { + zbc = new ZBClient() +}) + +afterEach(async () => { + try { + if (wf?.processInstanceKey) { + await zbc.cancelProcessInstance(wf.processInstanceKey) + } + } catch (e: any) { + // console.log('Caught NOT FOUND') // @DEBUG + } finally { + await zbc.close() // Makes sure we don't forget to close connection + } +}) + +test('Decrements the retries count by default', () => + new Promise(async done => { + await zbc.deployProcess('./src/__tests__/testdata/Worker-Failure-Retries.bpmn') + wf = await zbc.createProcessInstance('worker-failure-retries', { + conditionVariable: true, + }) + let called = false + + const worker = zbc.createWorker({ + taskType: 'service-task-worker-failure-retries', + taskHandler: job => { + if (!called) { + expect(job.retries).toBe(100) + called = true + return job.fail('Some reason') + } + expect(job.retries).toBe(99) + done(null) + return job.complete().then(async res => { + await worker.close() + return res + }) + } + }) + }) +) + +test('Set the retries to a specific number when provided with one via simple signature', () => + new Promise(async done => { + await zbc.deployProcess('./src/__tests__/testdata/Worker-Failure-Retries.bpmn') + wf = await zbc.createProcessInstance('worker-failure-retries', { + conditionVariable: true, + }) + let called = false + + const worker = zbc.createWorker({ + taskType: 'service-task-worker-failure-retries', + taskHandler: job => { + if (!called) { + expect(job.retries).toBe(100) + called = true + return job.fail('Some reason', 101) + } + expect(job.retries).toBe(101) + done(null) + return job.complete().then(async res => { + await worker.close() + return res + }) + } + }) + }) +) + +test('Set the retries to a specific number when provided with one via object signature', () => + new Promise(async done => { + await zbc.deployProcess('./src/__tests__/testdata/Worker-Failure-Retries.bpmn') + wf = await zbc.createProcessInstance('worker-failure-retries', { + conditionVariable: true, + }) + let called = false + + const worker = zbc.createWorker({ + taskType: 'service-task-worker-failure-retries', + taskHandler: job => { + if (!called) { + expect(job.retries).toBe(100) + called = true + return job.fail({ errorMessage: 'Some reason', retries: 101}) + } + expect(job.retries).toBe(101) + done(null) + return job.complete().then(async res => { + await worker.close() + return res + }) + } + }) + }) +) diff --git a/src/__tests__/integration/Worker-Failure.spec.ts b/src/__tests__/integration/Worker-Failure.spec.ts index 7d8e286..bc35331 100644 --- a/src/__tests__/integration/Worker-Failure.spec.ts +++ b/src/__tests__/integration/Worker-Failure.spec.ts @@ -164,3 +164,11 @@ test('Fails a process when the handler throws and options.failProcessOnException }, 1500) } })) + +test('Decrements the retries count by default', () => { + +}) + +test('Set the retries to a specific number when provided with one', () => { + +}) diff --git a/src/__tests__/testdata/Worker-Failure-Retries.bpmn b/src/__tests__/testdata/Worker-Failure-Retries.bpmn new file mode 100644 index 0000000..1485854 --- /dev/null +++ b/src/__tests__/testdata/Worker-Failure-Retries.bpmn @@ -0,0 +1,55 @@ + + + + + SequenceFlow_0fp53hs + + + + + + + + + SequenceFlow_0fp53hs + SequenceFlow_112zghv + + + + SequenceFlow_112zghv + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/lib/ZBWorkerBase.ts b/src/lib/ZBWorkerBase.ts index aa58ee7..1f8bc62 100644 --- a/src/lib/ZBWorkerBase.ts +++ b/src/lib/ZBWorkerBase.ts @@ -278,6 +278,11 @@ export class ZBWorkerBase< thisJob: ZB.Job ): ZB.JobCompletionInterface & ZB.JobCompletionInterface { let methodCalled: string | undefined + + /** + * This is a wrapper that allows us to throw an error if a job acknowledgement function is called more than once, + * for these functions should be called once only (and only one should be called, but we don't handle that case). + * */ const errorMsgOnPriorMessageCall = ( thisMethod: string, wrappedFunction: any @@ -298,26 +303,22 @@ You should call only one job action method in the worker handler. This is a bug return wrappedFunction(...args) } } + const cancelWorkflow = (job: ZB.Job) => () => this.zbClient .cancelProcessInstance(job.processInstanceKey) .then(() => ZB.JOB_ACTION_ACKNOWLEDGEMENT) const failJob = (job: ZB.Job) => ( - errorMessageOrFailureConfig: string | ZB.JobFailureConfiguration, + conf: string | ZB.JobFailureConfiguration, retries?: number ) => { - const errorMessage = - typeof errorMessageOrFailureConfig === 'string' - ? errorMessageOrFailureConfig - : (errorMessageOrFailureConfig as ZB.JobFailureConfiguration) - .errorMessage - const retryBackOff = - typeof errorMessageOrFailureConfig === 'string' - ? 0 - : (errorMessageOrFailureConfig as ZB.JobFailureConfiguration) - .retryBackOff ?? 0 - return this.failJob({ job, errorMessage, retries, retryBackOff }) + const isFailureConfig = (_conf: string | ZB.JobFailureConfiguration): _conf is ZB.JobFailureConfiguration => + typeof _conf === 'object' + const errorMessage = isFailureConfig(conf) ? conf.errorMessage : conf + const retryBackOff = isFailureConfig(conf) ? conf.retryBackOff ?? 0 : 0 + const _retries = isFailureConfig(conf) ? conf.retries ?? 0 : retries + return this.failJob({ job, errorMessage, retries: _retries, retryBackOff }) } const succeedJob = (job: ZB.Job) => (completedVariables?: T) =>