[
https://issues.apache.org/jira/browse/CLOUDSTACK-6242?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13956015#comment-13956015
]
Ding Yuan commented on CLOUDSTACK-6242:
---------------------------------------
Ping. Is there anything else needed from my side that can help on fixing these
cases?
> 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
> Attachments: CLOUDSTACK-6242.patch
>
>
> 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)
