[ https://issues.apache.org/jira/browse/CURATOR-515?focusedWorklogId=218013&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-218013 ]
ASF GitHub Bot logged work on CURATOR-515: ------------------------------------------ Author: ASF GitHub Bot Created on: 25/Mar/19 13:55 Start Date: 25/Mar/19 13:55 Worklog Time Spent: 10m Work Description: BELUGABEHR commented on pull request #308: CURATOR-515: Remove use of deprecated Throwables method URL: https://github.com/apache/curator/pull/308 ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking ------------------- Worklog Id: (was: 218013) Time Spent: 10m Remaining Estimate: 0h > Backgrounding CheckError > ------------------------- > > Key: CURATOR-515 > URL: https://issues.apache.org/jira/browse/CURATOR-515 > Project: Apache Curator > Issue Type: Improvement > Reporter: David Mollitor > Assignee: Jordan Zimmerman > Priority: Minor > Time Spent: 10m > Remaining Estimate: 0h > > This code is confusing to me: > > {code:java|title=Backgrounding.java} > void checkError(Throwable e, Watching watching) throws Exception > { > if ( e != null ) > { > if ( errorListener != null ) > { > errorListener.unhandledError("n/a", e); > } > else if ( e instanceof Exception ) > { > throw (Exception)e; > } > else > { > Throwables.propagate(e); > } > } > } > {code} > > > [https://github.com/apache/curator/blob/master/curator-framework/src/main/java/org/apache/curator/framework/imps/Backgrounding.java#L123-L140] > I *think* the code here is meaning to take a run-time Exception and wrap it > in a checked Exception. However, that is not actually happening here. > If the {{Throwable}} argument is an {{Exception}}, it is thrown as-is. Fair > enough. However, if the {{Throwable}} is a {{RuntimeException}} it is also > thrown here because {{RuntimeException}} is a sub-class of {{Exception}}. It > is not turned into a checked exception. So, if the {{Throwable}} is not an > {{Exception}}, the only other sub-class of {{Throwable}} is an {{Error}}. The > call {{Throwables.propagate(e)}} will will see that it is an {{Error}} and > simply throw it. > [https://docs.oracle.com/javase/8/docs/api/java/lang/RuntimeException.html] > [https://github.com/google/guava/blob/master/guava/src/com/google/common/base/Throwables.java#L132-L134] > So, really, whatever the {{Throwable}} argument is, it is simply re-thrown. > This code could be simplified to: > {code:java|title=Backgrounding.java} > void checkError(Throwable e, Watching watching) throws Exception > { > if ( e != null ) > { > if ( errorListener != null ) > { > errorListener.unhandledError("n/a", e); > } > else if ( e instanceof Exception ) > { > throw (Exception)e; > } > else > { > throw (Error)e; > } > } > } > {code} > Is this the intended behavior our should {{RuntimeException}} and {{Error}} > be wrapped into a checked {{Error}}? -- This message was sent by Atlassian JIRA (v7.6.3#76005)