asterixdb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From abdullah alamoudi <bamou...@gmail.com>
Subject Re: Catch Exception instead of Throwable
Date Thu, 13 Apr 2017 19:59:10 GMT
I have been switching positions too myself but now I think that catching Throwable is fine
in some cases. For example

try{
  writer.open()
} catch(Throwable th){
  writer.fail();
  throw th;
} finally{
  writer.close();
}

If we replace Throwable with Exception in this case, then we have a small problem and that
is writer.fail() will not be called in some cases. One of the common uses of writer.fail()
is to tell the writers in the pipeline not to do any additional work in the close() call since
the pipeline has failed and additional work could produce further failures.

So I think such uses is acceptable but other than using it to enforce the contract, we should
not catch Throwable.

Another way to think about this is to say that Errors are always terminal and that it is time
to call the shutdown hook and that we don't care if writer.fail() was not called in that case
and we probably assume that writer.close will encounter a few additional failures. If we choose
to follow that, then we should document this somewhere for reference and then stick to it
everywhere.

Personally, I prefer to catch Throwable in restricted places but I am okay with going route
2 as well.
~Abdullah.

> On Apr 13, 2017, at 8:02 PM, Till Westmann <tillw@apache.org> wrote:
> 
> Hi,
> 
> I just looked at Abdullah’s change [1] which adds more consistency around
> the behavior of our operators in error cases. In the change there are a
> number of SQ complaints about catching Throwable [2] and as I always
> wondered what the right approach on this is (changing my mind a few times),
> I’d like to discuss this here and capture the result to apply the outcome
> consistently across the system.
> 
> Please reply to this with your thoughts on catching Throwable vs not
> catching Error.
> 
> Thanks,
> Till
> 
> [1] https://asterix-gerrit.ics.uci.edu/#/c/1618
> [2] https://asterix-sonar.ics.uci.edu/coding_rules#rule_key=squid%3AS1181


Mime
View raw message