DaanHoogland commented on a change in pull request #2510: CLOUDSTACK-10334: Fix
inadequate information for handling catch clauses
URL: https://github.com/apache/cloudstack/pull/2510#discussion_r177093840
##########
File path: server/src/main/java/com/cloud/api/dispatch/ParamProcessWorker.java
##########
@@ -258,11 +258,11 @@ public void processParameters(final BaseCmd cmd, final
Map params) {
}
} catch (final IllegalArgumentException e) {
- s_logger.error("Error initializing command " +
cmd.getCommandName() + ", field " + field.getName() + " is not accessible.");
+ s_logger.error("Error initializing command " +
cmd.getCommandName() + ", field " + field.getName() + " is not accessible.", e);
Review comment:
I do not see that point, sorry. An unexpected error is caught and logged
(not rethrown) and than a more generic message/exception is thrown. Other than
unifying the two catch clauses in a catch
(IllegalArgumentException|IllegalAccessException e), I don't see the harm here.
We could improve by not logging at all and adding the original exception as a
root cause to the CloudRuntimeException. Is that what you mean?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
