[CURATOR-515] Backgrounding CheckError

[jira-importer]

This code is confusing to me:

 

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);
    }
}
    }
 

 
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:

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;
    }
}
    }
 

Is this the intended behavior our should RuntimeException and Error be wrapped into a checked Error?

---

Originally reported by belugabehr, imported from: Backgrounding CheckError

• assignee: randgalt
• status: Open
• priority: Minor
• resolution: Unresolved
• imported: 2025-01-21

[jira-importer]

belugabehr:

And the reason this caught my eye in the first place is that Throwables.propagate is deprecated.