Remove not needed IOException call

[henning-gerhardt]

Forward port of #6696

[matthias-ronge]

This pull request breaks the error handling on the call to ProcessBuilder.start() in Command lines 44−74. From the documentation of processBuilder.start():

Starting an operating system process is highly system-dependent. Among the many things that can go wrong are:
• The operating system program file was not found.
• Access to the program file was denied.
• The working directory does not exist.
In such cases an exception will be thrown. The exact nature of the exception is system-dependent, but it will always be a subclass of IOException.

Because the CommandInterface does not support throwing IOExceptions, and modifying interfaces was not desirable at the time, this solution was implemented. A cleanup is certainly commendable, but it must be done in a way that preserves functionality.

Possible solutions that preserve functionality could be:

• wrapping the IOException in Command in an UncheckedIOException and unwrapping it in CommandService
• modifying the CommandInterface so that IOExceptions can be passed directly, and then removing the catch, wrap, and unwrap code.

I also noticed the inconsistency, because related, although not part of this pull request, that FileManagement.createSymLink() returns boolean false on exceptions, which is not checked when it is called in WebDav.downloadToHome(). There, only the IOException is handled, but it is not passed up from createSymLink().

[matthias-ronge]

This code restores an IOException that is caught in Command lines 70−72 and wrapped in a string because the CommandInterface does not allow throwing an IOException. It must not be removed without a replacement, or failing scripts will no longer produce errors.

[henning-gerhardt]

There are here even more issues:

• there is no check if there are two messages and if there is only one message an IndexOutOfBondException (or similar) is thrown.
• the IOException is even thrown if an execution was successful and contain the word IOException. There is no check if the execution itself was failing or not and in my opinion the exception should only be thrown if there was a failure execution of the script.
• even if this exception is thrown is get caught some time later, an error message got logged but the user get the information that the execution was successful even there was an error!

[henning-gerhardt]

You are right, @matthias-ronge . I followed a wrong execution path as I thought this will be executed on any script execution. But this is not the case. This code is only executed if a file related script like the createSymlink or deleteSymlink scripts are executed.

You found or mentioned already some issues with the code in general, I added some more in your comment.

Should I open one or more issues with the new issues?

[solth]

@henning-gerhardt & @matthias-ronge if I understand this correctly we shouldn't merge this pull request, as it would break some behavior, correct? I would suggest to close the pull request (and it's back ports) then and open issues for the newly found issues.

[henning-gerhardt]

I will close this pull request after I opened the issues for the new founded issues.

[henning-gerhardt]

I opened 2 new issues

• IndexOutOfBoundsException while executing commands #6818 for the general IndexOutOfBoundsException
• FileManagement CommandService throws IOException on text output #6819 for the relay on text output IOException which throws this exception than independent of the script execution itself
• For not reporting issues while execution of commands in FileManagement Module I'm unsure if there are an UI response is wanted or not and @matthias-ronge did even report this too in his review of this pull request