Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Protect against bad contains/eq implementations in imported modules #397

Closed
bitprophet opened this issue Aug 19, 2011 · 3 comments
Closed

Protect against bad contains/eq implementations in imported modules #397

bitprophet opened this issue Aug 19, 2011 · 3 comments
Labels
Milestone

Comments

@bitprophet
Copy link
Member

@bitprophet bitprophet commented Aug 19, 2011

Description

The classic-task test includes an expression, <object> in [list of Fabric API functions]. This explodes if <object> has a bad implementation of __eq__(), such as in psycopg2, whose STRING member is a type which attempts to force-cast to int in its __eq__(). The result is a very perplexing traceback claiming that one of the function objects we're testing against is an invalid literal for "int". (A simple example of this can be found here.)

I think a sensible defense against this would be to wrap that statement in try/except: ValueError, since most broken __eq__ implementations probably raise ValueError, and certainly this one -- a widespread module many Fabric users are likely to be using -- does. Would prefer not to do a blanket exception here as that could still mask other, fabfile or Fabric level bugs.


Originally submitted by Jeff Forcier (bitprophet) on 2011-07-27 at 05:44pm EDT

@ghost ghost assigned bitprophet Aug 19, 2011
@kbd
Copy link

@kbd kbd commented Sep 12, 2011

Here's a stack trace and test code for this issue: http://paste.pocoo.org/show/474750/

@bitprophet
Copy link
Member Author

@bitprophet bitprophet commented Sep 12, 2011

@kbd's reproduction of this problem seems to raise TypeError, not ValueError, which is a bit odd, but hopefully we can trap both without widening the "capturing real errors" net too much.

bitprophet added a commit that referenced this issue Nov 23, 2011
@bitprophet
Copy link
Member Author

@bitprophet bitprophet commented Nov 23, 2011

FWIW I got TypeError too when testing on Linux (where it was easier for me to install the real-world test case, psycopg2). I've set up the fix to check for both, and hopefully there are no "actually broken" __eq__ implementations that raise either of these exception classes, since this fix will swallow them.

bitprophet added a commit that referenced this issue Nov 23, 2011
bitprophet added a commit that referenced this issue Nov 23, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants