Catch Exception instead of Throwable

[Till Westmann <ti...@apache.org>]

Hi,

I just looked at Abdullah\u2019s 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\u2019d 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

Re: Catch Exception instead of Throwable

[abdullah alamoudi <ba...@gmail.com>]

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 <ti...@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