From: "schmurfy (Julien A)" <schmurfy@...>
Date: 2013-07-15T20:12:28+09:00
Subject: [ruby-core:56013] [ruby-trunk - Feature #7688] Error hiding with rb_rescue() on Comparable#==, #coerce and others


Issue #7688 has been updated by schmurfy (Julien A).


I just got bitten by this problem and I agree there should not be any hidden rescue misleading you on what is really happening in your code, that's an horrible idea....
As pointed you don't usually except an exception to be raised anywhere in the code that's why they are called exceptions after all, I don't see why == should be any different since it's just a method like any other.

I think the bare minimum is to add a big or even huge warning in the Comparable module documentation so anyone who find this behavior stupid can use something else instead.

PS: If I sound a little aggressive that's because I just spent half an hour looking for a problem which was not at all where I was looking thanks to this hidden rescue...
----------------------------------------
Feature #7688: Error hiding with rb_rescue() on Comparable#==, #coerce and others
https://bugs.ruby-lang.org/issues/7688#change-40504

Author: Eregon (Benoit Daloze)
Status: Open
Priority: Normal
Assignee: matz (Yukihiro Matsumoto)
Category: core
Target version: next minor


Hello,

I believe error hiding is harmful because very dangerous
(it forgets errors which is likely unexpected) and hard to debug.

But I guess the compatibility is the main reason to keep these cases.

In the cases of Comparable#== and #coerce, I believe it is not worth to be compatible with this dangerous behavior
as it will at worse help to discover bugs in #<=> and #coerce methods which would raise an exception.

I doubt anyone rely on this and the #coerce spec (see #7645) itself makes me think this is unexpected behavior.
It would also simplify the spec, and no specific #coerce behavior would be needed to be defined as it would behave as a simple method call without edge cases.

So I think rb_rescue() or rb_rescue2(..., GenericErrorClass, ...) should be avoided if possible.
I analyzed these in the code base and it is used in a couple places:
* compar.c in cmp_equal(): this is the main offender in this regard with #coerce
* numeric.c in rb_num_coerce_{cmp,relop}() which call do_coerce(,,FALSE): This is the current subject of #7645.

* io.c in io_close(): to avoid raising if #close fails, which is likely less problematic,
  although it would be nicer to rescue only IO-related errors and warn when an exception is raised.
* range.c in range_init(): this is to provide a clearer error. I think it would be nice to show the original error as well.

Removing the general rescue in cmp_equal() revealed a couple bugs in RDoc due to this problem. I guess there are many others in the wild.

Can we please remove this anti-pattern?
I believe impact is only positive and that it should be done as soon as possible.

What do you think?


-- 
http://bugs.ruby-lang.org/