[ 
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)

Reply via email to