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

Support Error catch events without error code (catch-all Error events) #3362

Closed
5 tasks
koevskinikola opened this issue Dec 15, 2022 · 14 comments · Fixed by #3527
Closed
5 tasks

Support Error catch events without error code (catch-all Error events) #3362

koevskinikola opened this issue Dec 15, 2022 · 14 comments · Fixed by #3527
Assignees
Labels
Camunda 8 Flags an issue as related to Camunda 8 enhancement New feature or request modeling
Milestone

Comments

@koevskinikola
Copy link
Member

koevskinikola commented Dec 15, 2022

Problem you would like to solve

With camunda/camunda#11126, Zeebe now supports Error catch events without an error code.

This enables users to model a specific response for a known escalation code, and a general response for unknown escalation codes.

Currently, Error catch events without error codes are not supported in the Modeler for Camunda 8 BPMN diagrams.

image

Proposed solution

Alternatives considered

/

Additional context

In Zeebe, the feature will be available with version 8.2.0-alpha3.

@koevskinikola koevskinikola added the enhancement New feature or request label Dec 15, 2022
@nikku nikku added modeling Camunda 8 Flags an issue as related to Camunda 8 labels Dec 15, 2022
@nikku
Copy link
Member

nikku commented Dec 15, 2022

@koevskinikola Thanks for opening the issue.

For clarity could you attach an example process that defines a global error?

@koevskinikola
Copy link
Member Author

koevskinikola commented Dec 15, 2022

Hey @nikku, of course.

Here's a process where the two possible cases of a global error catch event are defined:

  1. The Error Start event of the Event sub process references an Error where no errorCode is defined.
  2. The Error Boundary event of the SubProcess doesn't define the errorRef attribute of the errorEventDefinition element.

Let me know if you need more details, or if there's a better way to attach a BPMN process on GitHub.

@nikku
Copy link
Member

nikku commented Dec 15, 2022

@koevskinikola You can attach by renaming to diagram.bpmn.txt.

Thanks for sharing the diagram. It is now clear to me that errorRef is optional, but also errorCode can be optional.

@nikku
Copy link
Member

nikku commented Dec 15, 2022

Updated the issue accordingly, please review @koevskinikola.

@koevskinikola
Copy link
Member Author

Looks good @nikku, thanks!

@nikku
Copy link
Member

nikku commented Dec 15, 2022

Moving to ready to potentially pick up this alignment topic in the next iteration(s).

@nikku nikku added the ready Ready to be worked on label Dec 15, 2022
@nikku nikku added this to the Next milestone Jan 10, 2023
@nikku nikku added backlog Queued in backlog and removed ready Ready to be worked on labels Jan 10, 2023
@CatalinaMoisuc CatalinaMoisuc added ready Ready to be worked on and removed backlog Queued in backlog labels Feb 7, 2023
@philippfromme philippfromme added the in progress Currently worked on label Feb 8, 2023 — with bpmn-io-tasks
@philippfromme philippfromme removed the ready Ready to be worked on label Feb 8, 2023
philippfromme added a commit to camunda/bpmnlint-plugin-camunda-compat that referenced this issue Feb 8, 2023
philippfromme added a commit to camunda/bpmnlint-plugin-camunda-compat that referenced this issue Feb 8, 2023
@philippfromme
Copy link
Contributor

@koevskinikola Can you help me understand what is not allowed. I'm not entirely sure about how scopes work. A couple of examples:

Example 1

case_1

Example 2

case_2

Example 3

case_3

Example 4

case_4

@koevskinikola
Copy link
Member Author

Hey @philippfromme,

From the examples you provided above:

  • Example 1 - not allowed. Event sub-processes are on the same scope as the root (process instance) scope. So two event sub-processes with a catch-all Error start event shouldn't be allowed since we need only one of them to catch all of the "un-assigned" errors.
  • Example 2 - allowed. The boundary catch-all Error event will catch all "un-assigned" errors in the scope of the sub-process Task (and any child scopes that it has). The catch-all Error start event will catch any "un-assigned" error events from the main process.
  • Example 3 - not allowed. Similar to example 1, the catch-all Error boundary events are assigned to the same Task scope, so any "un-assigned" errors that bubble up from the Task scope, or any child scopes only need to be caught by one of these events, the second is unnecessary.
  • Example 4 - allowed. The catch-all Error boundary events are assigned to different Task scopes. So each will catch "un-assigned" errors of the Task scope (and any child scopes) of the Task it borders on. So they aren't in conflict.

I can't think of any other use cases, so I think you managed to cover all of them. :)

Let me know if I can be of more help.

philippfromme added a commit to camunda/bpmnlint-plugin-camunda-compat that referenced this issue Feb 9, 2023
philippfromme added a commit to camunda/bpmnlint-plugin-camunda-compat that referenced this issue Feb 10, 2023
philippfromme added a commit to camunda/bpmnlint-plugin-camunda-compat that referenced this issue Feb 10, 2023
philippfromme added a commit to camunda/bpmnlint-plugin-camunda-compat that referenced this issue Feb 10, 2023
philippfromme added a commit to camunda/bpmnlint-plugin-camunda-compat that referenced this issue Feb 10, 2023
philippfromme added a commit to camunda/bpmnlint-plugin-camunda-compat that referenced this issue Feb 10, 2023
philippfromme added a commit to camunda/bpmnlint-plugin-camunda-compat that referenced this issue Feb 27, 2023
philippfromme added a commit to camunda/bpmnlint-plugin-camunda-compat that referenced this issue Feb 27, 2023
philippfromme added a commit to camunda/bpmnlint-plugin-camunda-compat that referenced this issue Feb 27, 2023
philippfromme added a commit to camunda/bpmnlint-plugin-camunda-compat that referenced this issue Feb 28, 2023
philippfromme added a commit to camunda/bpmnlint-plugin-camunda-compat that referenced this issue Feb 28, 2023
merge-me bot pushed a commit to camunda/bpmnlint-plugin-camunda-compat that referenced this issue Mar 1, 2023
@philippfromme philippfromme added fixed upstream Requires integration of upstream change and removed in progress Currently worked on labels Mar 2, 2023
@koevskinikola
Copy link
Member Author

@nikku @philippfromme I've been writing the Zeebe docs for this feature, and while going through our test coverage I realized I gave you the wrong answer here:

It is now clear to me that errorRef is optional, but also errorCode can be optional.

If we use our BPMN model API to model a catch-all error event, the XML output for a catch-all error event without an errorRef and without an errorCode will be the same, i.e. at the end an errorRef is not added. This is an example XML:

<boundaryEvent attachedToRef="task" id="error" name="error">
  <errorEventDefinition id="errorEventDefinition_bc91ed16-c948-4fc4-a201-fa7933fcb672"/>
</boundaryEvent>

From my understanding, for the Modeler this means that you don't need to make the errorCode optional.

@nikku
Copy link
Member

nikku commented Mar 17, 2023

From my understanding, for the Modeler this means that you don't need to make the errorCode optional.

This sounds like the more reasonable thing, if we don't have additional Zeebe specific things maintained on the error in the catch all case.

On the other hand, what happens if I deploy an error without an errorCode? Will it also have catch all semantics? We'd want to decide if we want to mirror the BPMN model API, or capabilities Zeebe has internally.

I'm always happy to go for the simpler solution.

@nikku nikku modified the milestones: 8.2, M63 Mar 21, 2023
@nikku
Copy link
Member

nikku commented Mar 22, 2023

CC @philippfromme.

Did you account for #3362 (comment)?

@CatalinaMoisuc
Copy link
Member

@philippfromme @nikku if this is fixed upstream, maybe we can pull out the remaining stuff in a separate issue so it is clear this is closed from Modeler side in the Product Hub issue.

@philippfromme
Copy link
Contributor

If we want to mirror the Zeebe engine behavior, we have to change the lint rules to always require an error code if an error reference exists but to not require an error reference. So we'd get rid of this error:

image

I'd say let's change it since the whole point of the linter is to ensure engine compatibility including quirks that might exist in the engine.

@philippfromme
Copy link
Contributor

@nikku camunda/bpmnlint-plugin-camunda-compat#89 changes the rules to mirror Zeebe engine behavior. Can you have a look?

philippfromme added a commit to camunda/bpmnlint-plugin-camunda-compat that referenced this issue Mar 27, 2023
@bpmn-io-tasks bpmn-io-tasks bot closed this as completed Mar 30, 2023
@bpmn-io-tasks bpmn-io-tasks bot removed the fixed upstream Requires integration of upstream change label Mar 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Camunda 8 Flags an issue as related to Camunda 8 enhancement New feature or request modeling
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants