fix(excel import): improved error messages shown in excel formula information#1036
Conversation
tom-rm-meyer-ISST
left a comment
There was a problem hiding this comment.
Thanks for the fix. Makes the software already much more reliable! :)
Found a few smaller needs for improvements.
…pdate-error-messages-excel-import
| if (ex.getCause() instanceof NotImplementedFunctionException){ | ||
| log.error("Runtime Exception is notImplementedFunction"); | ||
| } | ||
| log.error("RUntime exception cause is: " + ex.getCause().toString()); |
There was a problem hiding this comment.
Sorry for the misunderstanding. The snippet has only been meant as an example for catching the inner cause. Please implement the triple check (else-ifs / switch) on NotImplementedFunctionException, FormulaParseException and a fallback, same as before. The error messages were much better!
tom-rm-meyer-ISST
left a comment
There was a problem hiding this comment.
Thanks for the update, please reincorporate some error handling.
tom-rm-meyer-ISST
left a comment
There was a problem hiding this comment.
Functionality is great, but spotted a typo in remaining log message.
Please fix and we're ready to go! :)
|
Hi Tom! The check kept failing so I decided to revert the changes with the file name, hope that's okay |
tom-rm-meyer-ISST
left a comment
There was a problem hiding this comment.
LGTM, thanks for iterating over this with me!
Yeah, user inputs with CodeQl are tedious. Feel free to discuss these with me. We use the PatternStore non-whitespace or whitespace related pattern in the controllers. On Service Level the check still raises vulnerabilities, but IMHO this is false positive!
improved error messages shown in excel formula information for excel import
Description
Fixes #1034
Pre-review checks
Please ensure to do as many of the following checks as possible, before asking for committer review: