[MGNLUI-5415] Break ChainedAction execution sequence on failure gracefully Created: 09/Oct/19  Updated: 10/Mar/21  Resolved: 10/Mar/21

Status: Closed
Project: Magnolia UI
Component/s: None
Affects Version/s: 6.2
Fix Version/s: None

Type: Task Priority: Neutral
Reporter: Rishab Dhar Assignee: Rishab Dhar
Resolution: Won't Fix Votes: 0
Labels: UI
Remaining Estimate: 5h
Time Spent: 22m
Original Estimate: Not Specified

Issue Links:
dependency
depends upon MGNLUI-5455 Refactor AbstractActionExector to cat... Accepted
Template:
Acceptance criteria:
Empty
Task DoR:
Empty
Date of First Response:

 Description   

The info.magnolia.ui.contentapp.action.ChainedAction#ChainedAction currently keeps executing actions irrespective of whether there is a failure (validation) in an Action.

In order to prevent further validation checks on dependent actions, failing actions have two choices - either duplicate validation logic or throw an Exception.

Throwing Exception propagates all the way to the parent UI as RpcInvocationException for a very simple thing such as form validation failure. In order to avoid this behavior, the action sequence has to duplicate this validation in subsequent actions, which has it's own side effects of code duplication and suboptimal performance due to dependent ActionDefinition objects created and executed each time the chain sequence is executed.



 Comments   
Comment by Roman Kovařík [ 09/Oct/19 ]

Hard to believe this as exceptions are not swallowed anywhere, so the other actions can't be executed.

Maybe the "failure" in your case is just failed validation which is not a "real" failure?

Comment by Rishab Dhar [ 09/Oct/19 ]

No the follow up Actions do not execute, but the problem is if I throw an Exception on validation failure, it shows RpcExecutionException on UI, which is quite misleading from UX perspective. Duplication of validation logic to prevent dependent action executions to prevent RpcInvocationException seems not optimal. Not to mention it leads to code duplication in a long chain of actions just for mere validation check and performance drop (n loop iterations where n is the number of chained actions defined).

Comment by Roman Kovařík [ 09/Oct/19 ]

No the follow up Actions do not execute,

Doesn't this statement contradict with the ticket description?

 it shows RpcExecutionException on UI

so it's a general problem, not a chained action problem right?

 Duplication of validation logic to prevent dependent action executions to prevent RpcInvocationException seems not optimal.

every action would probably validate different things (e.g. first action validates the form, whereas second validates e.g. if there is new item created to operate on...)

Comment by Rishab Dhar [ 09/Oct/19 ]

Doesn't this statement contradict with the ticket description?

What I meant is validation failure in the chain, not exception failure. My assumption was that let's say I have validation check in an Action, but not in dependent Actions, the chain continues executing, dependent actions. This like I said in my previous comment can be solved in two ways currently which I mentioned. Neither one of which is currently optimal.

 so it's a general problem, not a chained action problem right?

Partially yes, because the ChainedAction is trying to execute ActionDefinitions but doesn't have the API necessary from info.magnolia.ui.contentapp.browser.ActionExecutionService because of which it has a custom ActionExecutor defined which directly invokes the ActionExecutor API to execute an Action, which does not handle Exceptions gracefully. As the ActionExecutionService was being delegated to do this graceful handling, ChainedAction should probably do this as well.

 @Override
 public final void execute(String actionName, Object... args) throws ActionExecutionException {
        try {
            Action action = createAction(actionName, args);
            action.execute();
        } catch (RuntimeException e) {
            throw new ActionExecutionException("Action execution failed for action: " + actionName, e);
        }
 }

 

every action would probably validate different things (e.g. first action validates the form, whereas second validates e.g. if there is new item created to operate on...)

If the form validation itself failed, it is redundant to check whether an item has been created. This just adds more complexity unnecessarily to mask the problem of non graceful handling at ChainedAction.

Comment by Roman Kovařík [ 09/Oct/19 ]

It looks like you are trying to use the very simple implementation chained action for your use case but nobody prevents you to implement/extend your custom chained action or implement one action instead of a chain. Wouldn't that be better...or why do you have to use the chained action while one actions depends on result of another other?

Generated at Mon Feb 12 09:26:17 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.