Repository: Bluetooth-Devices/dbus-fast (language: python)

Issue:
Cython's issue with python3.9 since dbus-fast v1.90.2
**The bug**
When a DBusError is raised from a method, the raise exception `_as_message`  seems to not be properly resolved from dbus_fast.message_bus.SendReply._exit. It leads to no answer ever being sent on the dbus-message.

The error does not happen without cython.

**To Reproduce**
With the following test file (which is a trimmed down version of the high-level service example from the doc):
```python
#test_bug.py
from dbus_fast import DBusError
from dbus_fast.aio import MessageBus
from dbus_fast.service import ServiceInterface, method


class ExampleInterface(ServiceInterface):
    def __init__(self):
        super().__init__('com.example.SampleInterface0')

    @method()
    async def fail(self):
        raise DBusError('com.example.name.fail', 'fail')


async def start_service():
    bus0 = await MessageBus().connect()
    iface = ExampleInterface()
    bus0.export('/com/example/sample0', iface)
    await bus0.request_name('com.example.name')


async def call_fail():
    bus = await MessageBus().connect()
    introspection = await bus.introspect('com.example.name',
                                         '/com/example/sample0')

    proxy_object = bus.get_proxy_object('com.example.name',
                                        '/com/example/sample0',
                                        introspection)
    interface = proxy_object.get_interface('com.example.SampleInterface0')
    await interface.call_fail()


async def startncall():
    await start_service()
    await call_fail()


if __name__ == "__main__":
    import asyncio
    asyncio.run(startncall())
else:
    import pytest

    @pytest.mark.asyncio
    async def test_fail_bug():
        with pytest.raises(DBusError):
            await startncall()
```

In a venv with dbus-fast >=1.90.2 run:
```bash
$ python3.9 test_bug.py
Exception in callback MessageBus._make_method_handler.<locals>._coroutine_method_handler.<locals>._done(<Task finishe...Error('fail')>) at /.cache/pypoetry/virtualenvs/dbus-fast-b0n82em1-py3.9/lib/python3.9/site-packages/dbus_fast/aio/message_bus.py:456
handle: <Handle MessageBus._make_method_handler.<locals>._coroutine_method_handler.<locals>._done(<Task finishe...Error('fail')>) at /.cache/pypoetry/virtualenvs/dbus-fast-b0n82em1-py3.9/lib/python3.9/site-packages/dbus_fast/aio/message_bus.py:456>
Traceback (most recent call last):
  File "/.cache/pypoetry/virtualenvs/dbus-fast-b0n82em1-py3.9/lib/python3.9/site-packages/dbus_fast/aio/message_bus.py", line 459, in _done
    result = fut.result()
  File "/misc/dbus-fast/test_bug.py", line 12, in fail
    raise DBusError('com.example.name.fail', 'fail')
dbus_fast.errors.DBusError: fail

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/lib/python3.9/asyncio/events.py", line 80, in _run
    self._context.run(self._callback, *self._args)
  File "/.cache/pypoetry/virtualenvs/dbus-fast-b0n82em1-py3.9/lib/python3.9/site-packages/dbus_fast/aio/message_bus.py", line 463, in _done
    send_reply(
  File "src/dbus_fast/message_bus.py", line 103, in dbus_fast.message_bus.SendReply.__exit__
    return self._exit(exc_type, exc_value, tb)
  File "src/dbus_fast/message_bus.py", line 84, in dbus_fast.message_bus.SendReply._exit
    self(exc_value._as_message(self._msg))
AttributeError: type object 'Exception' has no attribute '_as_message'
```

The same problem happens with `pytest test_bug.py`, however i cannot trigger the issue when running with `poetry run pytest` as if the corresponding dbus-fast was without cython compilation.

**Additional context**

Only reproduced with cython and with python3.9 (not python3.11). 

It is not caught by the tests suite because it seems the task_method testsuite only run in non-cython mode. (Or something else is happening, i do not know really well how poetry works).

The bug happens since v1.90.2 but is not linked to this update only to the pipy binary: i guess the cython version used to compile changed at this point (I did not manage to track down the culprit cython version)



Discussion / hints:
I haven't had any luck reproducing this issue.

Can you make sure that you don't have any old dbus-fast installs on the system? Maybe strace python to make sure it's loading the right cext?

I'll keep trying 
Maybe related https://github.com/cython/cython/issues/5511
Confirmed the test suite is running the cythonized version. 

Will write some units for SendReply in the hopes that it fails 
i'm going to write a Dockerfile to reproduce it.
awesome. thanks!
I'm pretty sure I got replication with the unit tests I just wrote.

Validating that now
I was able to get it to happen with mismatched cext from an older version but not after a clean recompile


I'm going to make a new container and try a fresh install with the tests I just merged and see if I can get it to happen
no dice, it still works as expected.

I'll wait for the docker file
here is a dockerfile with pypi's version:
```Dockerfile
# bug_cython.Dockerfile
FROM docker.io/debian:bullseye
COPY test_bug.py /tmp
RUN apt-get update -y && apt-get install -y dbus dbus-x11 python3-pip systemd

#XXX: trick to get dbus running in containers
RUN dbus-uuidgen > /var/lib/dbus/machine-id && mkdir -p /var/run/dbus && echo $'\#!/bin/bash\n\
dbus-daemon --config-file=/usr/share/dbus-1/system.conf --print-address\n\
export $(dbus-launch) \n\
bash -c "$@"' >> /tmp/dbus_run.sh && chmod +x /tmp/dbus_run.sh

RUN pip3 install "dbus-fast==1.90.1"
RUN /tmp/dbus_run.sh "python3 /tmp/test_bug.py" || echo "done"
RUN pip3 install "dbus-fast==1.90.2"
RUN /tmp/dbus_run.sh "python3 /tmp/test_bug.py" || echo "done"
```

The run of it (with the initial steps cached for readability) gives:
```bash
$ podman build -f bug_cython.Dockerfile .       2023-08-24 15:26:35 mocramis pts/0
STEP 1/8: FROM docker.io/debian:bullseye
STEP 2/8: COPY test_bug.py /tmp
--> Using cache 4df095337faa8ef8c45063f85b9c0b874ea48b376c77a72ce2cd16bdf0f64281
--> 4df095337faa
STEP 3/8: RUN apt-get update -y && apt-get install -y dbus dbus-x11 python3-pip systemd
--> Using cache 7858a866a27a6a1e3862c882c2033d69fe2a4d106664512e983c31a8b6403b2e
--> 7858a866a27a
STEP 4/8: RUN dbus-uuidgen > /var/lib/dbus/machine-id && mkdir -p /var/run/dbus && echo $'\#!/bin/bash\ndbus-daemon --config-file=/usr/share/dbus-1/system.conf --print-address\nexport $(dbus-launch) \nbash -c "$@"' >> /tmp/dbus_run.sh && chmod +x /tmp/dbus_run.sh
--> 1e2e2f461881
STEP 5/8: RUN pip3 install "dbus-fast==1.90.1"
Collecting dbus-fast==1.90.1
  Downloading dbus_fast-1.90.1-cp39-cp39-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (4.8 MB)
Installing collected packages: dbus-fast
Successfully installed dbus-fast-1.90.1
--> 7feee3032093
STEP 6/8: RUN /tmp/dbus_run.sh "python3 /tmp/test_bug.py" || echo "done"
/tmp/dbus_run.sh: 1: $#!/bin/bash: not found
unix:path=/run/dbus/system_bus_socket,guid=0470cdd801265657bd80c39664e75a93
Traceback (most recent call last):
  File "/tmp/test_bug.py", line 41, in <module>
    asyncio.run(startncall())
  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 "/tmp/test_bug.py", line 36, in startncall
    await call_fail()
  File "/tmp/test_bug.py", line 31, in call_fail
    await interface.call_fail()
  File "/usr/local/lib/python3.9/dist-packages/dbus_fast/aio/proxy_object.py", line 108, in method_fn
    BaseProxyInterface._check_method_return(msg, intr_method.out_signature)
  File "/usr/local/lib/python3.9/dist-packages/dbus_fast/proxy_object.py", line 80, in _check_method_return
    raise DBusError._from_message(msg)
dbus_fast.errors.DBusError: fail
done
WARN[0005] archive: skipping "/home/mocramis/.local/share/containers/storage/overlay/6bb499b0ff41abb6980883877bc6ae926b7287d8527ed2a6cf8582f25fed41d7/diff/run/dbus/system_bus_socket" since it is a socket
WARN[0005] archive: skipping "/home/mocramis/.local/share/containers/storage/overlay/6bb499b0ff41abb6980883877bc6ae926b7287d8527ed2a6cf8582f25fed41d7/diff/tmp/dbus-4P5OxAbvMN" since it is a socket
--> 779fd4b8b52c
STEP 7/8: RUN pip3 install "dbus-fast==1.90.2"
Collecting dbus-fast==1.90.2
  Downloading dbus_fast-1.90.2-cp39-cp39-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (4.7 MB)
Installing collected packages: dbus-fast
  Attempting uninstall: dbus-fast
    Found existing installation: dbus-fast 1.90.1
    Uninstalling dbus-fast-1.90.1:
      Successfully uninstalled dbus-fast-1.90.1
Successfully installed dbus-fast-1.90.2
--> a0b0c49f527e
STEP 8/8: RUN /tmp/dbus_run.sh "python3 /tmp/test_bug.py" || echo "done"
/tmp/dbus_run.sh: 1: $#!/bin/bash: not found
dbus-daemon[4]: Failed to start message bus: The pid file "/run/dbus/pid" exists, if the message bus is not running, remove this file
Exception in callback MessageBus._make_method_handler.<locals>._coro_method_handler.<locals>.done(<Task finishe...Error('fail')>) at /usr/local/lib/python3.9/dist-packages/dbus_fast/aio/message_bus.py:416
handle: <Handle MessageBus._make_method_handler.<locals>._coro_method_handler.<locals>.done(<Task finishe...Error('fail')>) at /usr/local/lib/python3.9/dist-packages/dbus_fast/aio/message_bus.py:416>
Traceback (most recent call last):
  File "/usr/local/lib/python3.9/dist-packages/dbus_fast/aio/message_bus.py", line 418, in done
    result = fut.result()
  File "/tmp/test_bug.py", line 12, in fail
    raise DBusError('com.example.name.fail', 'fail')
dbus_fast.errors.DBusError: fail

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/lib/python3.9/asyncio/events.py", line 80, in _run
    self._context.run(self._callback, *self._args)
  File "/usr/local/lib/python3.9/dist-packages/dbus_fast/aio/message_bus.py", line 422, in done
    send_reply(
  File "src/dbus_fast/message_bus.py", line 83, in dbus_fast.message_bus.SendReply.__exit__
  File "src/dbus_fast/message_bus.py", line 64, in dbus_fast.message_bus.SendReply._exit
AttributeError: type object 'Exception' has no attribute '_as_message'
````
(The build hand there. Note the missing "done" at the end)

The result is the same if i replace 1.90.2 by 1.93.0

This should prove the issue's existence. I'll work on a version with some compiling  from source now.
```

FROM docker.io/debian:bullseye
COPY test_bug.py /tmp
RUN apt-get update -y && apt-get install -y dbus dbus-x11 python3-pip systemd git

#XXX: trick to get dbus running in containers
RUN dbus-uuidgen > /var/lib/dbus/machine-id && mkdir -p /var/run/dbus && echo $'\#!/bin/bash\n\
dbus-daemon --config-file=/usr/share/dbus-1/system.conf --print-address\n\
export $(dbus-launch) \n\
bash -c "$@"' >> /tmp/dbus_run.sh && chmod +x /tmp/dbus_run.sh

ENV REQUIRE_CYTHON=1

RUN pip3 install "git+https://github.com/Bluetooth-Devices/dbus-fast.git@send_reply"
RUN /tmp/dbus_run.sh "python3 /tmp/test_bug.py" || echo "done"
```

This worked for me to replicate it

Fix patch:
diff --git a/src/dbus_fast/message_bus.pxd b/src/dbus_fast/message_bus.pxd
index fc39e7d5..137971bc 100644
--- a/src/dbus_fast/message_bus.pxd
+++ b/src/dbus_fast/message_bus.pxd
@@ -18,11 +18,6 @@ cdef object assert_bus_name_valid
 cdef _expects_reply(Message msg)
 
 
-cdef class SendReply:
-
-    cdef object _bus
-    cdef object _msg
-
 cdef class BaseMessageBus:
 
     cdef public object unique_name
diff --git a/src/dbus_fast/message_bus.py b/src/dbus_fast/message_bus.py
index 0403edda..2fe9af79 100644
--- a/src/dbus_fast/message_bus.py
+++ b/src/dbus_fast/message_bus.py
@@ -3,7 +3,6 @@
 import socket
 import traceback
 import xml.etree.ElementTree as ET
-from types import TracebackType
 from typing import Any, Callable, Dict, List, Optional, Type, Union
 
 from . import introspection as intr
@@ -21,6 +20,7 @@
 from .errors import DBusError, InvalidAddressError
 from .message import Message
 from .proxy_object import BaseProxyObject
+from .send_reply import SendReply
 from .service import ServiceInterface, _Method
 from .signature import Variant
 from .validators import assert_bus_name_valid, assert_object_path_valid
@@ -57,55 +57,6 @@ def _block_unexpected_reply(reply: _Message) -> None:
 BLOCK_UNEXPECTED_REPLY = _block_unexpected_reply
 
 
-class SendReply:
-    """A context manager to send a reply to a message."""
-
-    __slots__ = ("_bus", "_msg")
-
-    def __init__(self, bus: "BaseMessageBus", msg: Message) -> None:
-        """Create a new reply context manager."""
-        self._bus = bus
-        self._msg = msg
-
-    def __enter__(self):
-        return self
-
-    def __call__(self, reply: Message) -> None:
-        self._bus.send(reply)
-
-    def _exit(
-        self,
-        exc_type: Optional[Type[Exception]],
-        exc_value: Optional[Exception],
-        tb: Optional[TracebackType],
-    ) -> bool:
-        if exc_value:
-            if isinstance(exc_value, DBusError):
-                self(exc_value._as_message(self._msg))
-            else:
-                self(
-                    Message.new_error(
-                        self._msg,
-                        ErrorType.SERVICE_ERROR,
-                        f"The service interface raised an error: {exc_value}.\n{traceback.format_tb(tb)}",
-                    )
-                )
-            return True
-
-        return False
-
-    def __exit__(
-        self,
-        exc_type: Optional[Type[Exception]],
-        exc_value: Optional[Exception],
-        tb: Optional[TracebackType],
-    ) -> bool:
-        return self._exit(exc_type, exc_value, tb)
-
-    def send_error(self, exc: Exception) -> None:
-        self._exit(exc.__class__, exc, exc.__traceback__)
-
-
 class BaseMessageBus:
     """An abstract class to manage a connection to a DBus message bus.
 
diff --git a/src/dbus_fast/send_reply.py b/src/dbus_fast/send_reply.py
new file mode 100644
index 00000000..e53f3da5
--- /dev/null
+++ b/src/dbus_fast/send_reply.py
@@ -0,0 +1,59 @@
+import traceback
+from types import TracebackType
+from typing import TYPE_CHECKING, Optional, Type
+
+from .constants import ErrorType
+from .errors import DBusError
+from .message import Message
+
+if TYPE_CHECKING:
+    from .message_bus import BaseMessageBus
+
+
+class SendReply:
+    """A context manager to send a reply to a message."""
+
+    __slots__ = ("_bus", "_msg")
+
+    def __init__(self, bus: "BaseMessageBus", msg: Message) -> None:
+        """Create a new reply context manager."""
+        self._bus = bus
+        self._msg = msg
+
+    def __enter__(self):
+        return self
+
+    def __call__(self, reply: Message) -> None:
+        self._bus.send(reply)
+
+    def _exit(
+        self,
+        exc_type: Optional[Type[Exception]],
+        exc_value: Optional[Exception],
+        tb: Optional[TracebackType],
+    ) -> bool:
+        if exc_value:
+            if isinstance(exc_value, DBusError):
+                self(exc_value._as_message(self._msg))
+            else:
+                self(
+                    Message.new_error(
+                        self._msg,
+                        ErrorType.SERVICE_ERROR,
+                        f"The service interface raised an error: {exc_value}.\n{traceback.format_tb(tb)}",
+                    )
+                )
+            return True
+
+        return False
+
+    def __exit__(
+        self,
+        exc_type: Optional[Type[Exception]],
+        exc_value: Optional[Exception],
+        tb: Optional[TracebackType],
+    ) -> bool:
+        return self._exit(exc_type, exc_value, tb)
+
+    def send_error(self, exc: Exception) -> None:
+        self._exit(exc.__class__, exc, exc.__traceback__)


Test patch:
diff --git a/tests/service/test_standard_interfaces.py b/tests/service/test_standard_interfaces.py
index bc137fcb..83ed6bc6 100644
--- a/tests/service/test_standard_interfaces.py
+++ b/tests/service/test_standard_interfaces.py
@@ -240,7 +240,6 @@ async def test_standard_interface_properties():
         "org.freedesktop.DBus.Peer",
         "org.freedesktop.DBus.ObjectManager",
     ]:
-
         result = await bus2.call(
             Message(
                 destination=bus1.unique_name,
diff --git a/tests/test_address_parser.py b/tests/test_address_parser.py
index c65d856d..848adc22 100644
--- a/tests/test_address_parser.py
+++ b/tests/test_address_parser.py
@@ -2,7 +2,6 @@
 
 
 def test_valid_addresses():
-
     valid_addresses = {
         "unix:path=/run/user/1000/bus": [("unix", {"path": "/run/user/1000/bus"})],
         "unix:abstract=/tmp/dbus-ft9sODWpZk,guid=a7b1d5912379c2d471165e9b5cb74a03": [
diff --git a/tests/test_marshaller.py b/tests/test_marshaller.py
index cf141cb5..a2ffcbf6 100644
--- a/tests/test_marshaller.py
+++ b/tests/test_marshaller.py
@@ -104,7 +104,6 @@ def test_unmarshalling_with_table(unmarshall_table):
     from dbus_fast._private import unmarshaller
 
     for item in unmarshall_table:
-
         stream = io.BytesIO(bytes.fromhex(item["data"]))
         unmarshaller = Unmarshaller(stream)
         try:
@@ -486,7 +485,6 @@ def tests_fallback_no_cython():
 
 
 def test_unmarshall_large_message():
-
     stream = io.BytesIO(bytes.fromhex(get_managed_objects_msg))
     unmarshaller = Unmarshaller(stream)
     unmarshaller.unmarshall()
diff --git a/tests/test_send_reply.py b/tests/test_send_reply.py
index a1228040..7982480f 100644
--- a/tests/test_send_reply.py
+++ b/tests/test_send_reply.py
@@ -1,15 +1,12 @@
 import os
-import traceback
-from types import TracebackType
-from typing import Any, Callable, Dict, List, Optional, Type, Union
-from unittest.mock import Mock
 
 import pytest
 
 from dbus_fast.constants import ErrorType, MessageType
 from dbus_fast.errors import DBusError
 from dbus_fast.message import Message
-from dbus_fast.message_bus import BaseMessageBus, SendReply
+from dbus_fast.message_bus import BaseMessageBus
+from dbus_fast.send_reply import SendReply
 
 
 @pytest.fixture(autouse=True)
@@ -44,7 +41,7 @@ def _setup_socket(self) -> None:
     )
     send_reply = SendReply(mock_message_bus, mock_message)
 
-    with send_reply as reply:
+    with send_reply:
         raise DBusError(ErrorType.DISCONNECTED, "Disconnected", None)
 
     assert len(messages) == 1