search

matrix-org / sydent

Sydent: Reference Matrix Identity Server
http://matrix.org
Apache License 2.0
303 stars 84 forks source link

Fix a RuntimeWarning exception raised when calling `ThreepidBinder._notify` without awaiting #552

Closed anoadragon453 closed 1 year ago

anoadragon453 commented 1 year ago

When an error occurred while informing a homeserver of a successful bind between a Matrix ID and a third-party ID, Sydent should log the error and retry the request to the homeserver.

Right now this is broken, and Sydent will raise a RuntimeWarning due to ThreepidBinder._notify not being await'd:

2023-02-28 11:41:47,501 - sydent.threepid.bind - 214 - WARNING - Error notifying on bind for @admin:localhost: Connection was refused by other side: 111: Connection refused. - rescheduling
/home/work/.cache/pypoetry/virtualenvs/matrix-sydent-RILnZfem-py3.10/lib/python3.10/site-packages/twisted/internet/base.py:991: RuntimeWarning: coroutine 'ThreepidBinder._notify' was never awaited
  call.func(*call.args, **call.kw)
RuntimeWarning: Enable tracemalloc to get the object allocation traceback

This would call the request to never be retried as well, breaking reliability.

This PR fixes the call to ThreepidBinder._notify so that it is properly awaited via defer.ensureDeferred. The self.sydent.reactor.callLater(...) call is used for backing off purposes, and expects a Callable, not a Coroutine, hence the use of something like defer.ensureDeferred.

anoadragon453 commented 1 year ago

Yes, we should write a test to ensure that homeserver callbacks on bind get retried. This was a drive-by fix though, and I'm not likely to get around to that in a while.

So I've left an issue instead: https://github.com/matrix-org/sydent/issues/554