[Twisted-Python] Ldaptor: [PATCH] Reroute errback to deferred returned by search()

The LDAPEntryWithClient.search() method used to send the LDAP request with a call like self.client.send_multiResponse(... ) send_multiResponse() returns a deferred that was just discarded. If the operation causes an error then the errback fired on the discarded deferred will remain unhandled. The deferred returned by search() will then not have any errback fired and the caller of search() will be waiting forever. This change adds an errback to the deferred returned by send_multiResponse() and has the error rerouted to the errback chain of the deferred returned by search(). --- Discussion: This hits right into the internals of Ldaptor and I am not sure I understand this well enough to make an accurate fix. The problem I encountered is real and can be reproduced, and this change does seem to work around it. Any comments and suggestions welcome! ldaptor/protocols/ldap/ldapsyntax.py | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-) diff --git a/ldaptor/protocols/ldap/ldapsyntax.py b/ldaptor/protocols/ldap/ldapsyntax.py index f54d2f8..471ffa6 100644 --- a/ldaptor/protocols/ldap/ldapsyntax.py +++ b/ldaptor/protocols/ldap/ldapsyntax.py @@ -683,7 +683,7 @@ class LDAPEntryWithClient(entry.EditableLDAPEntry): typesOnly=typesOnly, filter=filterObject, attributes=attributes) - self.client.send_multiResponse( + dsend = self.client.send_multiResponse( op, self._cbSearchMsg, d, cb, complete=not attributes, sizeLimitIsNonFatal=sizeLimitIsNonFatal) @@ -692,6 +692,11 @@ class LDAPEntryWithClient(entry.EditableLDAPEntry): else: if callback is None: d.addCallback(lambda dummy: results) + def rerouteerr(e): + d.errback(e) + # returning None will stop the error + # from being propagated and logged. + dsend.addErrback(rerouteerr) return d def lookup(self, dn): -- 1.7.1.5.g49342

On Wed, Sep 1, 2010 at 4:17 PM, Itamar Turner-Trauring <itamar@itamarst.org> wrote:
On Wed, 2010-09-01 at 14:52 +0300, Anton Gyllenberg wrote:
Any comments and suggestions welcome!
Unit tests are always a good idea :)
That is so true. And if not writing new ones, then at least running existing ones. That I hadn't done and it seems I actually broke something. Any ideas or directions on how to fix the issue with search not handling errors in send_multiResponse(). Thanks! Anton

On Wed, Sep 1, 2010 at 5:05 PM, Anton Gyllenberg <anton@iki.fi> wrote:
On Wed, Sep 1, 2010 at 4:17 PM, Itamar Turner-Trauring <itamar@itamarst.org> wrote:
On Wed, 2010-09-01 at 14:52 +0300, Anton Gyllenberg wrote:
Any comments and suggestions welcome!
Unit tests are always a good idea :)
That is so true. And if not writing new ones, then at least running existing ones. That I hadn't done and it seems I actually broke something.
The send_multiResponse() method within the test framework worked differently and returned None instead of a deferred like the real version. Making it return defer.Deferred() instead made the tests pass. I'm new to this trial stuff so need to do some reading before I dare make test cases for new code. Thanks! Anton
participants (2)
-
Anton Gyllenberg
-
Itamar Turner-Trauring