cuba-platform / cuba

CUBA Platform is a high level framework for enterprise applications development
https://www.cuba-platform.com
Apache License 2.0
1.34k stars 220 forks source link

BackgroundWorkWindow affects how exceptions from background task are handled #2520

Open dkz opened 5 years ago

dkz commented 5 years ago

Environment

Description of the bug or enhancement

BackgroundWorkWindow wraps tasks with com.haulmont.cuba.gui.backgroundwork.LocalizedTaskWrapper which completely disregards com.haulmont.cuba.web.exception.ExceptionHandlers: exception is reported immediately despite the fact it can have exception handlers registered in the app.

Expected behavior

Sample A.

AppBeans.get(BackgroundWorker.class)
    .handle(new VerificationTask(authRequest, validationContext))
    .execute();

underlying task does getDsContext().commit() and violates unique constraint. As a result a message is shown to user with appropriate localized constrain violation string, i.e. "Job with the same contract number already exists".

Actual behavior

Sample B, this sample should be equivalent to the A in terms of exception handling:

BackgroundWorkWindow.show(new VerificationTask(authRequest, validationContext),
    getMessage("creditCard.verificationBgTask.title"),
    getMessage("creditCard.bgTransaction.message"), false);

actually: underlying task does getDsContext().commit() and after constraint violation a "Report an exception" dialog is shown with an exception stack trace.

Flaurite commented 4 years ago

Hi @dkz! Unfortunately, I cannot reproduce the issue. Do you use BackgroundTask#handleException() for handling exception in the UI thread? Please, try this example: demo.zip

dkz commented 4 years ago

Hi @Flaurite, Sorry, there is an important detail I missed in original ticket description: commit happens in done() method. Thus my complaint in regard of LocalizedTaskWrapper is directed towards implementation of done() and cancelled() method with try-catch block: exception is caught and is shown in a non-user-friendly fashion in comparison to globally defined application exception handlers.

Flaurite commented 4 years ago

Sorry for the long delay @dkz, LocalizedTaskWrapper has such default behavior for done() and result() methods to do not miss the exception if it will occur. If we do not show the exception and delegate it to handleException() the lambda in window.closeAndRun() will not be invoked. In your case, you need to manually handle exception in your done() and result() methods and if it necessary delegate to ExceptionHandlers:

try {
    dataManager.commit(commitContext);
} catch (Exception e) {
    App.getInstance().getExceptionHandlers().handle(new ErrorEvent(e));
}