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

Hidden exception in synchronous execution of DelegateCommand #442

Closed
AKreisel opened this issue Oct 4, 2016 · 3 comments
Closed

Hidden exception in synchronous execution of DelegateCommand #442

AKreisel opened this issue Oct 4, 2016 · 3 comments
Assignees
Labels
Milestone

Comments

@AKreisel
Copy link

AKreisel commented Oct 4, 2016

If I have a DelegateCommand that does not run in background and my Action throws an exception I only get the log message "Exception in Command Execution" and no further information.

This is because of the lines in DelegateCommand.java:

private void callActionAndSynthesizeServiceRun() {
    try {
        getStateObjectPropertyReadable().setValue(State.SCHEDULED);

        // We call the User Action. If an exception occurs we save it, therefore we can use it in the Test
        // (createSynthesizedTask) to throw it during the Service invocation.
        actionSupplier.get().action();

        getStateObjectPropertyReadable().setValue(State.SUCCEEDED);
    } catch (Exception e) {
        LOG.error("Exception in Command Execution", occuredException);
        this.occuredException = e;
        getStateObjectPropertyReadable().setValue(State.FAILED);
    }
}

The field "occuredException" is logged before it is initialized. Also I cannot use the "getException()" method of the Service interface to get the Throwable object, so I have no information about the problem.

@manuel-mauky
Copy link
Collaborator

Hi,
thanks for reporting this bug.

At first we have to fix the log statement so that the correct exception appears in the logs.
After that we have to provide the exception to the user. Maybe we should change the occurredException from a basic field to a ObjectProperty<Exception> and provide getter/setter/property-accessors. This would be similar to the getException method from Service.

@AKreisel Do you think this would be a suitable way of handling the exception or do you have other ideas for the API to handle exceptions?

@sialcasa can you please have a look at this issue. Is there any argument against introducing a exception property for the user?

@manuel-mauky manuel-mauky added this to the 1.5.3 milestone Oct 5, 2016
@sialcasa
Copy link
Owner

sialcasa commented Oct 5, 2016

@lestard On the first view we would have to implement it like the stateProperty because the exceptionProperty is already provided by the extended Service in the DelegateCommand.

/**
     * For internal purposes we need to change the state property of the service.
     */
    private ObjectProperty<State> getStateObjectPropertyReadable() {
        return (ObjectProperty<State>) stateProperty();
    }

@manuel-mauky
Copy link
Collaborator

Ok let's do it :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants