Ding Yuan created CLOUDSTACK-6242:
-------------------------------------
Summary: Potential bugs and improvements for exception handlers
Key: CLOUDSTACK-6242
URL: https://issues.apache.org/jira/browse/CLOUDSTACK-6242
Project: CloudStack
Issue Type: Bug
Security Level: Public (Anyone can view this level - this is the default.)
Components: Projects, Storage Controller
Affects Versions: 4.2.0
Reporter: Ding Yuan
Hi Cloudstack developers,
I'm reporting a few cases where the exception handling logic could be improved.
In particular, there are a few cases where the caught exception is too general
(over-catch), and/or missing log messages. Attaching a patch for review.
There are a few cases where it looks suspicious but I do not know how to fix
right now (all filenames and line numbers are based on version 4.2.1):
=========================
Case 1:
Line: 375, File: "com/cloud/network/ovs/OvsTunnelManagerImpl.java"
{noformat}
326: try {
327: String myIp = getGreEndpointIP(dest.getHost(), nw);
.. .. ..
373: } catch (Exception e) {
374: // I really thing we should do a better handling of these
exceptions
375: s_logger.warn("Ovs Tunnel network created tunnel failed", e);
376: }
{noformat}
Comment seems to suggest for a better handling.
=========================
=========================
Case 2:
Line: 2295, File: "com/cloud/resource/ResourceManagerImpl.java"
{noformat}
2284: try {
2285: /*
2286: * FIXME: this is a buggy logic, check with alex.
Shouldn't
2287: * return if propagation return non null
2288: */
2289: Boolean result =
_clusterMgr.propagateResourceEvent(h.getId(),
ResourceState.Event.UpdatePassword);
2290: if (result != null) {
2291: return result;
2292: }
2293:
2294: doUpdateHostPassword(h.getId());
2295: } catch (AgentUnavailableException e) {
2296: }
{noformat}
Seem from the comment the logic should be fixed.
A similar code snipeet is at:
Line: 2276, File: "com/cloud/resource/ResourceManagerImpl.java"
=========================
=========================
Case 3:
Line: 184, File:
"org/apache/cloudstack/api/command/user/autoscale/CreateAutoScaleVmGroupCmd.java"
{noformat}
174: try
175: {
176: success = _autoScaleService.configureAutoScaleVmGroup(this);
177: if (success) {
178: vmGroup = _entityMgr.findById(AutoScaleVmGroup.class,
getEntityId());
179: AutoScaleVmGroupResponse responseObject =
_responseGenerator.createAutoScaleVmGroupResponse(vmGroup);
180: setResponseObject(responseObject);
181: responseObject.setResponseName(getCommandName());
182: }
183: } catch (Exception ex) {
184: // TODO what will happen if Resource Layer fails in a step
inbetween
185: s_logger.warn("Failed to create autoscale vm group", ex);
186: }
{noformat}
The comment, "TODO", seems to suggest for better handling.
=========================
=========================
Case 4:
Line: 222, File: "com/cloud/api/ApiDispatcher.java"
This snippet is moved to ParamProcessWorker.java in the trunk.
{noformat}
203: try {
204: setFieldValue(field, cmd, paramObj, parameterAnnotation);
.. ..
220: } catch (CloudRuntimeException cloudEx) {
221: s_logger.error("CloudRuntimeException", cloudEx);
222: // FIXME: Better error message? This only happens if the
API command is not executable, which typically
223: //means
224: // there was
225: // and IllegalAccessException setting one of the
parameters.
226: throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR,
"Internal error executing API command " + cmd.getCommandName().substring(0,
cmd.getCommandName().length() - 8));
227: }
{noformat}
The "FIXME" comment seems to suggest for getter error message.
=========================
--
This message was sent by Atlassian JIRA
(v6.2#6252)
