Return from `@method()` decorator

jonasBoss commented 2 years ago

The wrapped method in service.method() (service.py line 91) does not return the value from the original method. This is confusing and limits the utility of the interface.

conciser the following:

class ExampleInterface(ServiceInterface):
    def __init__(self):
        super().__init__('com.example.name')

    @method()
    def do_something(self) -> 's':
        return "done"

    @method()
    def do_more_things(self, times: 'i') -> 'as':
        return [self.do_something() for _ in range(times)]

    @method()
    def do_it_another_way(self, times: 'i') -> 'as':
        return [self.do_it() for _ in range(times)]

    def do_it(self) -> 's':
        return "done"

calling do_more_things will raise a error:

ERROR:root:got unexpected error processing a message: Cannot serialize Python type "None".
Traceback (most recent call last):
  File "/venv/lib/python3.8/site-packages/dbus_next/message_bus.py", line 621, in _on_message
    self._process_message(msg)
  File "/venv/lib/python3.8/site-packages/dbus_next/message_bus.py", line 712, in _process_message
    handler(msg, send_reply)
  File "/venv/lib/python3.8/site-packages/dbus_next/message_bus.py", line 734, in handler
    send_reply(Message.new_method_return(msg, method.out_signature, body, fds))
  File "/venv/lib/python3.8/site-packages/dbus_next/message_bus.py", line 637, in __call__
    bus.send(reply)
  File "/venv/lib/python3.8/site-packages/dbus_next/aio/message_bus.py", line 326, in send
    self._writer.schedule_write(msg, future)
  File "/venv/lib/python3.8/site-packages/dbus_next/aio/message_bus.py", line 82, in schedule_write
    self.buffer_message(msg, future)
  File "/venv/lib/python3.8/site-packages/dbus_next/aio/message_bus.py", line 78, in buffer_message
    (msg._marshall(negotiate_unix_fd=self.negotiate_unix_fd), copy(msg.unix_fds), future))
  File "/venv/lib/python3.8/site-packages/dbus_next/message.py", line 215, in _marshall
    body_block = Marshaller(self.signature, self.body)
  File "/venv/lib/python3.8/site-packages/dbus_next/_private/marshaller.py", line 8, in __init__
    self.signature_tree.verify(body)
  File "/venv/lib/python3.8/site-packages/dbus_next/signature.py", line 359, in verify
    type_.verify(body[i])
  File "/venv/lib/python3.8/site-packages/dbus_next/signature.py", line 287, in verify
    self._verify_array(body)
  File "/venv/lib/python3.8/site-packages/dbus_next/signature.py", line 229, in _verify_array
    child_type.verify(member)
  File "/venv/lib/python3.8/site-packages/dbus_next/signature.py", line 259, in verify
    raise SignatureBodyMismatchError('Cannot serialize Python type "None"')
dbus_next.errors.SignatureBodyMismatchError: Cannot serialize Python type "None"

adding a return statement seems to fix this issue:

    @no_type_check_decorator
    def decorator(fn):
        @wraps(fn)
        def wrapped(*args, **kwargs):
             return fn(*args, **kwargs) # <-- here

        fn_name = name if name else fn.__name__
        wrapped.__dict__['__DBUS_METHOD'] = _Method(fn, fn_name, disabled=disabled)

        return wrapped

JohnFreeborg commented 2 years ago

This is a fantastic improvement.

My use case that requires it is pytest. I have a class that has been exposed as a ServiceInterface, but I still want to be able to use pytest to directly create it and call methods on it without going through dbus. Without this added 'return', the called methods always just return None.

Now I can easily have some pytests use dbus and others use the class directly and it all still works.

jonasBoss commented 1 year ago

since there is no movement on this PR: #123 Here is a fix which can be deployed without touching the dbus-next library

def fix_non_returning_decorator(decorator):
    def fixed_decorator(fn):
        retval = None

        @decorator
        @functools.wraps(fn)
        def non_returning_fn(*args, **kwargs):
            nonlocal retval
            retval = fn(*args, **kwargs)
            return retval  # this return gets eaten by the decorator

        @functools.wraps(non_returning_fn)
        def returning_fn(*args, **kwargs):
            non_returning_fn(*args, **kwargs)
            return retval

        return returning_fn

    return fixed_decorator

class MyAwesomeInterface(service.ServiceInterface):

    @fix_non_returning_decorator(service.method())
    def cool_method() -> 's':
        return "Hi there!"

Unfortunately this breaks async functions.

jonasBoss commented 1 year ago

Better workaround, which does not break async functions:

This does rely a bit on dbus-next internals.

from dbus_next import service

def method(name: str = None, disabled: bool = False):
    # this is a workaround for https://github.com/altdesktop/python-dbus-next/issues/119
    @typing.no_type_check_decorator
    def fixed_decorator(fn):
        # we don't actually decorate the function
        # dbus-next only cares about the __dict__
        fn.__dict__ = service.method(name, disabled)(fn).__dict__
        return fn

    return fixed_decorator

altdesktop / python-dbus-next

Return from `@method()` decorator #119