`modify_password` fails in asyncio when a new password is provided

[morian]

Hi!

I am now trying to use modify_password in async mode, which works fine when no new password is provided. Unfortunately, this raises a strange error in async mode:

Traceback (most recent call last):
  File "/home/lab/bonsai/update_me.py", line 60, in <module>
    asyncio.run(asynchronous(oldpass, newpass))
  File "/usr/lib/python3.9/asyncio/runners.py", line 44, in run
    return loop.run_until_complete(main)
  File "/usr/lib/python3.9/asyncio/base_events.py", line 642, in run_until_complete
    return future.result()
  File "/home/lab/bonsai/update_me.py", line 42, in asynchronous
    res = await conn.modify_password(
  File "/home/lab/bonsai/venv/lib/python3.9/site-packages/bonsai/asyncio/aioconnection.py", line 60, in _poll
    raise exc
  File "/home/lab/bonsai/venv/lib/python3.9/site-packages/bonsai/asyncio/aioconnection.py", line 55, in _poll
    return await asyncio.wait_for(fut, timeout)
  File "/usr/lib/python3.9/asyncio/tasks.py", line 442, in wait_for
    return await fut
  File "/home/lab/bonsai/venv/lib/python3.9/site-packages/bonsai/asyncio/aioconnection.py", line 40, in _ready
    res = super().get_result(msg_id)
bonsai.errors.InvalidMessageID: Given message ID is invalid or the associated operation is already finished. (0xFF9C [-100])

When a new password is provided, the server is expected to return no data, which is handled here: https://github.com/noirello/bonsai/blob/034ec671b60e41f1181a64ea4a7fe3d1dff7967a/src/_bonsai/ldapconnection.c#L1080

In this situation, None is returned as the result to the python caller.

In async mode, a result None means that the caller has to try again later as described here: https://github.com/noirello/bonsai/blob/034ec671b60e41f1181a64ea4a7fe3d1dff7967a/src/bonsai/asyncio/aioconnection.py#L41

This can be fixed by returning True or any other value instead of None, but that would break the user API and existing applications. On the other hand, rewriting the way these loops work may be a lot of work :-(.

Regards,

Romain

[morian]

I also noticed during the debugging that _ready() is called continuously due to the fact that we listen to the socket POLLOUT events through the writer: https://github.com/noirello/bonsai/blob/034ec671b60e41f1181a64ea4a7fe3d1dff7967a/src/bonsai/asyncio/aioconnection.py#L38

I am not sure whether this is needed here, but can confirm that it is called appropriately when these writers are removed. This has little to do with the original issue here but still can be worth enough to be mentioned.

[noirello]

Thank you. I think that was an oversight from my part. This method should return True instead of None, when no new password is returned, just like the add and delete methods.

[morian]

I did a commit based on version 1.3.0 for future references: https://github.com/morian/bonsai/commit/74efefa69e05cae72f5aa6acedb20f156393759f

It seems like the tests do not check for the return value.

[noirello]

Great. Can I create PR from your patch?

[morian]

Sure, go ahead if you think it fully covers the issue!