#2207 - Investigate Root Cause of Camunda Workers Getting Stuck

[ann-aot]

• The issue was able to replicate in one of the workers when we submitted an application with a long income, which was an error in the worker logic during the assessment. However, the worker is not getting any acknowledgment from the code and it is waiting

• So, the next time a new assessment comes, it gets stuck in the same worker for an infinite time.

• checked our worker global error handle in https://vscode.dev/github/bcgov/SIMS/blob/fix/sims-%232207/sources/packages/backend/apps/workers/src/zeebe/zeebe-transport-strategy.ts#L57
  
  and found that, the return await jobHandler(job); is not throwing any error, when there is an error, as a result, the catch is not invoked and the worker is not getting the acknowledgment.

• fixed the global handler, the jobhandler is returning an observable, so, added a logic to await for it when the return type is observable,
  

• And added try-catch, and if there is any error in the controller level, the code acknowledges the worker as a failed job, so that the worker is free and can focus on the next jobs.

• This causes an incident in the Camunda, Which developers can take a look at (Before this fix, Camunda never caused an incident for these scenarios).

-Replicate

1. To replicate the issue, I decreased the maxJobsToActivate: to 1 and decreased the timeout to 10.
   
2. And created/edited 2 applications with a lengthy income,
   
3. which made the create income verification worker stuck in both the case
   
4. And then, when you submit an application with a proper income, the new assessment is also stuck in the same worker.
5. After the fix, step 2 will create incidents in Camunda, and step 4, will work as expected.
   Step 2, after the fix
   
   
   
   Step 4, after the fix,
   

Ref: https://docs.camunda.io/docs/components/best-practices/development/writing-good-workers/#nodejs-client

[andrepestana-aot]

Looks good! Great job investigating the bug!

[dheepak-aot]

Thanks for doing the testing @ann-aot to find out that global error handler wasn't working and we had a dev discussion this morning and agreed on the point that we must fix the global error handler instead of enforcing developer to explicitly add the try catch everywhere.

Will continue the review after the global error handler fix.

[andrewsignori-aot]

Thanks for doing the testing @ann-aot to find out that the global error handler wasn't working we had a dev discussion this morning and agreed on the point that we must fix the global error handler instead of enforcing the developer to explicitly add the try-catch everywhere.

Will continue the review after the global error handler fix.

The global error handler seems to be failing because an unhandled expectation inside the Zeebe worker method would be handled by the rpc-exceptions-handler.ts and this will return an Observable that is not expected currently.

To test the theory, if we change the global handler as below it will report the job.fail properly and we will able to see the retries counter going down and the Camunda incident generated after 3 attempts.

Still, if the unhandled exception is not of the type RpcExecption Nest will generate a generic "Internal Server Error" exception message that will be sent to Camunda. The log with the proper exception will be logged in the worker's POD but Camunda will receive only the generic message.

IMO we need to fix the global error handler and also talk about the possibility to ensure the controllers will always provide a better error handling.
@dheepak-aot @guru-aot @andrepestana-aot @sh16011993 FYI

[andrewsignori-aot]

Typo on "seting".

[andrewsignori-aot]

Great research and findings. Please let's talk about having the global error handler fixed and also I would like to check if the default timeouts are actually woking.

[andrewsignori-aot]

Because we would be repeating this along all controller and jobs, I was expecting that a basic helper would be created, as shown below.

} catch (error: unknown) {
  return createUnexpectedJobFail(error, job);
}

The helper method can be something like below.

createUnexpectedJobFail(
    error: unknown,
    job: ZeebeJob,
    logger?: Logger,
  ): MustReturnJobActionAcknowledgement {
    const jobLogger = logger ?? new Logger(job.type);
    const errorMessage = `Unexpected error while processing job. ${parseException(error)}`;
    jobLogger.error(errorMessage);
    return job.fail(errorMessage);
  }

As an out-of-PR-scope suggestion, I believe that we can create the below error formatter to avoid logging as jobLogger.log(${error.name} ${error.message});.

parseException(error: unknown): string {
  return JSON.stringify(error, null, 2);
}

[andrewsignori-aot]

This would a generic method to be used every where, not only by workers. It can be moved to the generic lib.

[andrewsignori-aot]

I was expecting that we no longer need line 103.
The helper would be called as return createUnexpectedJobFail(error, job, jobLogger);

[dheepak-aot]

Thanks for comments. 👍

[dheepak-aot]

Great work @ann-aot and great team effort putting our heads together to resolve this issue. Added few minor comments.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[github-actions]

Backend Unit Tests Coverage Report

Totals
Statements:	17.94% ( 2170 / 12094 )
Methods:	8.2% ( 126 / 1537 )
Lines:	20.8% ( 1906 / 9163 )
Branches:	9.9% ( 138 / 1394 )

[github-actions]

E2E Workflow Workers Coverage Report

Totals
Statements:	47.47% ( 300 / 632 )
Methods:	40% ( 32 / 80 )
Lines:	51.86% ( 251 / 484 )
Branches:	25% ( 17 / 68 )

[github-actions]

E2E Queue Consumers Coverage Report

Totals
Statements:	72.5% ( 406 / 560 )
Methods:	63.38% ( 45 / 71 )
Lines:	74.53% ( 357 / 479 )
Branches:	40% ( 4 / 10 )

[dheepak-aot]

Thanks for making the changes 👍

[github-actions]

E2E SIMS API Coverage Report

Totals
Statements:	53.44% ( 3900 / 7298 )
Methods:	49.89% ( 470 / 942 )
Lines:	58.41% ( 3181 / 5446 )
Branches:	27.36% ( 249 / 910 )

[andrewsignori-aot]

Thanks for doing the changes, looks good 👍

[guru-aot]

LGTM