curator-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Work logged] (CURATOR-515) Backgrounding CheckError
Date Mon, 25 Mar 2019 13:56:00 GMT

     [ 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:
users@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)

Mime
View raw message