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

Align deployment rejection message for DMN resources #8806

Closed
Tracked by #571
saig0 opened this issue Feb 17, 2022 · 7 comments · Fixed by #17475
Closed
Tracked by #571

Align deployment rejection message for DMN resources #8806

saig0 opened this issue Feb 17, 2022 · 7 comments · Fixed by #17475
Assignees
Labels
area/ux Marks an issue as related to improving the user experience component/engine component/zeebe Related to the Zeebe component/team hacktoberfest Marks an issue as a candidate to be a Hacktoberfest contribution kind/bug Categorizes an issue or PR as a bug scope/broker Marks an issue or PR to appear in the broker section of the changelog version:8.4.7 Marks an issue as being completely or in parts released in 8.4.7 version:8.6.0-alpha1 Label that represents issues released on verions 8.6.0-alpha1

Comments

@saig0
Copy link
Member

saig0 commented Feb 17, 2022

Describe the bug

If I deploy an invalid DMN resource then the deployment is rejected. The rejection message may look like the following example:

/bin/zbctl deploy ~/IdeaProjects/zeebe/engine/src/test/resources/processes/invalid_process.bpmn ~/IdeaProjects/zeebe/dmn/src/test/resources/decision-table-with-invalid-expression.dmn
Error: rpc error: code = InvalidArgument desc = Command 'CREATE' rejected with code 'INVALID_ARGUMENT': Expected to deploy new resources, but encountered the following errors:
'/home/philipp/IdeaProjects/zeebe/engine/src/test/resources/processes/invalid_process.bpmn': - Element: process
    - ERROR: Must have at least one start event

FEEL unary-tests: failed to parse expression '"blue': Expected (negation | positiveUnaryTests | anyInput):1:1, found "\"blue"

The rejection message for BPMN and DMN looks different. The rejection contains the failure message of the DMN but it is missing the related deployment resource. Without the resource, it could be hard to identify the failure.

To Reproduce

Deploy the following resources from the Zeebe repo.

/bin/zbctl deploy ~/IdeaProjects/zeebe/engine/src/test/resources/processes/invalid_process.bpmn ~/IdeaProjects/zeebe/dmn/src/test/resources/decision-table-with-invalid-expression.dmn

Expected behavior

The rejection message for DMN resources contains the name of the resource. Similar to the message for BPMN resources.

Error: rpc error: code = InvalidArgument desc = Command 'CREATE' rejected with code 'INVALID_ARGUMENT': Expected to deploy new resources, but encountered the following errors:
'/home/philipp/IdeaProjects/zeebe/engine/src/test/resources/processes/invalid_process.bpmn': - Element: process
    - ERROR: Must have at least one start event

'~/IdeaProjects/zeebe/dmn/src/test/resources/decision-table-with-invalid-expression.dmn': FEEL unary-tests: failed to parse expression '"blue': Expected (negation | positiveUnaryTests | anyInput):1:1, found "\"blue"

Log/Stacktrace

None.

Environment:

  • OS:
  • Zeebe Version: 1.4.0-alpha1
  • Configuration:
@saig0 saig0 added kind/bug Categorizes an issue or PR as a bug scope/broker Marks an issue or PR to appear in the broker section of the changelog labels Feb 17, 2022
@saig0 saig0 added this to the Evaluate DMN decisions milestone Feb 17, 2022
@saig0 saig0 added this to Planned in Zeebe Feb 17, 2022
@KerstinHebel KerstinHebel removed this from Planned in Zeebe Mar 23, 2022
@korthout korthout added the area/ux Marks an issue as related to improving the user experience label Aug 30, 2022
@korthout
Copy link
Member

Marking priority as later because DMN improvements are not a main priority for the process automation right now.

This issue may be interesting to pick up alongside the epics #8083 and #9864 (https://github.com/camunda/product-hub/issues/69)

Please comment if you think this should have a higher priority.

@saig0 saig0 added the hacktoberfest Marks an issue as a candidate to be a Hacktoberfest contribution label Sep 29, 2022
@romansmirnov romansmirnov added the component/zeebe Related to the Zeebe component/team label Mar 5, 2024
@jfriedenstab
Copy link

Hi @korthout, I'm currently working on process applications in Web Modeler (which will basically enable bundled deployments of multiple files) and stumbled across this issue. If the deployment bundle contains more than one invalid DMN diagram, you can't tell from the Zeebe response which error belongs to which file. So it would be nice if the DMN resource name could be included in the response (in the same way as it's already done for BPMN diagrams and forms).

@megglos
Copy link
Contributor

megglos commented Apr 15, 2024

ZPA-Triage:

  • @jfriedenstab is this blocking the epic? if so is there any deadline you need this to be ready?
  • for forms we append the resource name
  • it's less of a bug but an inconsistency/limitation that makes it hard to link errors with the correct resource
  • should be part of the related epic https://github.com/camunda/product-hub/issues/1983
    ==> given it's done for BPMN and forms already, seems a quick fix

@jfriedenstab
Copy link

Hi @megglos,

@jfriedenstab is this blocking the epic? if so is there any deadline you need this to be ready?

The Web Modeler epic is planned for 8.6.0-alpha1 (and on SaaS we'd like to release the feature even before if possible), so it would be nice if the problem could be fixed on the Zeebe side as soon as possible 🙃 (I just noticed that there's already a PR with a fix – great! 🎉).
I'd say it's not necessarily blocking for us; it would just make the user experience a bit worse if we couldn't clearly indicate which DMN diagram has an error.

@megglos
Copy link
Contributor

megglos commented Apr 17, 2024

@jfriedenstab fix is ready and will be part of 8.6.0-alpha1 #17475

@megglos
Copy link
Contributor

megglos commented Apr 17, 2024

@jfriedenstab do you also need this to be present in patches for previous minor releases or is 8.6 enough?

@jfriedenstab
Copy link

jfriedenstab commented Apr 17, 2024

@megglos If it's easily possible, it would be nice to backport this at least to 8.4 and 8.5 (as we will support process application deployments to clusters >= 8.4).

github-merge-queue bot pushed a commit that referenced this issue Apr 18, 2024
## Description

Included the resource name in the rejection message of DMN validation
error. Also, updated the related test to verify the expected behaviour.

## Related issues

closes #8806
github-merge-queue bot pushed a commit that referenced this issue Apr 18, 2024
…urces (#17582)

# Description
Backport of #17475 to `stable/8.4`.

relates to #8806
original author: @berkaycanbc
@Zelldon Zelldon added version:8.4.7 Marks an issue as being completely or in parts released in 8.4.7 version:8.6.0-alpha1 labels May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ux Marks an issue as related to improving the user experience component/engine component/zeebe Related to the Zeebe component/team hacktoberfest Marks an issue as a candidate to be a Hacktoberfest contribution kind/bug Categorizes an issue or PR as a bug scope/broker Marks an issue or PR to appear in the broker section of the changelog version:8.4.7 Marks an issue as being completely or in parts released in 8.4.7 version:8.6.0-alpha1 Label that represents issues released on verions 8.6.0-alpha1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants