Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Handling error situation in multi-instance call activities when in async completion job #3264

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

BorisKlinkerSAP
Copy link

@BorisKlinkerSAP BorisKlinkerSAP commented Apr 5, 2022

The flowable-engine dispatches a JOB_EXECUTION_FAILURE event when an exception is thrown during the EndExecutionOperation of a process instance. The failing job is converted to a DeadLetterJob.

We use a CallActivity to trigger (one or multiple) SubProcess instance(s). We have an event listener which handles failing subprocess instances.

We discovered that the context of the JOB_EXECUTION_FAILURE event is set differently depending on the asyncComplete flag of the CallActivity:

  • In case asyncComplete=false the JOB_EXECUTION_FAILURE event is triggered in the context of the subprocess execution, the DeadLetterJob belongs to the subprocess execution.
  • When asyncComplete=true the JOB_EXECUTION_FAILURE event is triggered in the context of the super process execution, the DeadLetterJob belongs to the super process execution.

From our perspective the JOB_EXECUTION_FAILURE event should always be dispatched in the context of the execution which caused the exception.
Currently this is the case for CallActivity triggered with asyncComplete=false, but not for asyncComplete=true.
Following PR corrects this behaviour.

Please let us know your opinion/feedback. Based on this we will finalise this PR.

@@ -67,8 +67,13 @@ protected static void populateEventWithCurrentContext(FlowableEngineEventImpl ev
if (persistedObject instanceof Job) {
Job jobObject = (Job) persistedObject;
if (jobObject.getScopeType() == null) {
event.setExecutionId(jobObject.getExecutionId());
event.setProcessInstanceId(jobObject.getProcessInstanceId());
if ("async-complete-call-actiivty".equals(jobObject.getJobHandlerType())) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be better to use the public static variable AsyncCompleteCallActivityJobHandler.TYPE instead of a literal string? At least it would hide the incorrect spelling of activity 😄

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be better to use the public static variable AsyncCompleteCallActivityJobHandler.TYPE instead of a literal string? At least it would hide the incorrect spelling of activity 😄

Hi @dbmalkovsky, thanks for your feedback. The typo in existing flowable code is quite unfortunate... excellent intention to cover it...
However, the change here is in FlowableJobEventBuilder which is located in maven module flowable-job-service, from here the AsyncCompleteCallActivityJobHandler class (and its constant) is not reachable as it is located in flowable-engine without dependency in that direction.

BorisKlinkerSAP and others added 2 commits September 8, 2022 10:10
…n-asynch-execution_TEST

Tests: failing subflow completion w/ completeAsync true/false
@BorisKlinkerSAP BorisKlinkerSAP marked this pull request as ready for review October 6, 2022 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants