[jira] [Commented] (CLOUDSTACK-6242) Potential bugs and improvements for exception handlers
[
https://issues.apache.org/jira/browse/CLOUDSTACK-6242?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13977153#comment-13977153
]
Ding Yuan commented on CLOUDSTACK-6242:
---
Please close this ticket since it has been fixed by Daan. Thanks all!
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)
[jira] [Commented] (CLOUDSTACK-6242) Potential bugs and improvements for exception handlers
[
https://issues.apache.org/jira/browse/CLOUDSTACK-6242?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13957655#comment-13957655
]
Ding Yuan commented on CLOUDSTACK-6242:
---
Thanks [~murali.reddy]! I have uploaded the patch to reviews.apache.org.
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)
[jira] [Commented] (CLOUDSTACK-6242) Potential bugs and improvements for exception handlers
[
https://issues.apache.org/jira/browse/CLOUDSTACK-6242?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=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)
[jira] [Created] (CLOUDSTACK-6242) Potential bugs and improvements for exception handlers
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)
[jira] [Updated] (CLOUDSTACK-6242) Potential bugs and improvements for exception handlers
[
https://issues.apache.org/jira/browse/CLOUDSTACK-6242?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Ding Yuan updated CLOUDSTACK-6242:
--
Attachment: CLOUDSTACK-6242.patch
Patch against trunk.
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)
