Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Async notifier does not handle errors #1865

Open
peufeu2 opened this issue Sep 25, 2024 · 0 comments
Open

Async notifier does not handle errors #1865

peufeu2 opened this issue Sep 25, 2024 · 0 comments
Labels

Comments

@peufeu2
Copy link

peufeu2 commented Sep 25, 2024

Greetings! I'm using python-can to talk to my inverter's battery.

Describe the bug

In class Notifier:

    def _on_message_available(self, bus: BusABC) -> None:
        if msg := bus.recv(0):
            self._on_message_received(msg)

When an exception occurs, for example the CAN interface going down or being unplugged, bus.recv() raises but the exception is not handled. The threaded version does call _on_error(), so I guess the async version should too. I fixed it by inheriting the class, but these 3 lines of code would be better in notifier.py.

class Notifier_e (can.Notifier):
    def _on_message_available(self, bus):
        try:
            super()._on_message_available(bus)
        except Exception as exc:
            if not self._on_error(exc):
                # If it was not handled, raise the exception here
                raise

Then _on_error() passes the exception to the listeners, for example AsyncBufferedReader, but it doesn't handle exceptions either: instead the coroutine waiting for messages simply stays stuck forever. So I extended it in the same way:

class AsyncBufferedReader_e( can.AsyncBufferedReader ):
    def on_error( self, exc ):
        self.buffer.put_nowait( exc )

    async def __anext__(self):
        m = await self.buffer.get()
        if isinstance( m, BaseException ):
            raise m
        return m

It puts the exception into the queue with messages, so when the coroutine waiting on these messages gets to the exception, it is raised, and the coroutine can process it. This preserves order between messages and exceptions, so messages received and pushed into the queue before the exception occurs will be processed.

I use it to cleanly disconnect the CAN interface, then connect again when it is plugged back into the USB port.

I'm not sure about Exception vs BaseException. I guess it would be useful to pass KeyboardInterrupt to the waiting coroutine, so it would be terminated in the same way as a Ctrl-C occurring during a blocking recv().

Have a nice day!

@peufeu2 peufeu2 added the bug label Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant