A bug bit me where I had code that looked like

urllib2.build_opener(func_returning_cookie_jar())

instead of

urllib2.build_opener(HTTPCookieProcessor(func_returning_cookie_jar())

the subsequent http request went through just fine, except it didn't send the cookie.  Nothing complained, except me, now.

Throwing the exception deeper in the code made the unittest fail, so I suspect the "silently ignore handlers which don't actually do any handling" behavior might be by design, but it seems terrible to me.

Patch #1752270 seems more idiomatic (TypeError, not ValueError) and mildly less intrusive (it does not change the return value of urllib2.OpenerDirector.add_handler()).  Also, your patch adds a backwards incompatibility which the new one does not: Currently a correct call to urllib2.build_opener() may return successfully without adding all of its handler arguments to the returned urllib2.OpenerDirector, since a urllib2.BaseHandler instance may have no appropriate methods (no .http_open(), etc. etc.).  Your patch causes build_opener() (but not .add_handler()) to raise ValueError in that case.  One could argue that it should raise an exception, but there is still scope for breakage, e.g. since these attributes may be added at runtime, and the number of added attributes may happen to be zero (and in fact, urllib2.ProxyHandler falls into this category).

I think the docs do not need to be updated if my revised patch is applied, since they already state that urllib2.OpenerDirector.add_handler() accepts only urllib2.BaseHandler instances (and typically the possibility of a TypeError being raised is not explicitly documented in stdlib docs).

Your patch looks good to me, I have no objection so long as this error will be caught instead of failing without complaint.  I am rejecting my patch in favor of yours.  Thanks for looking into this.

Committed #1752270.