search

johnmaguire / Cardinal

A Python IRC bot, designed to make adding functionality quick and simple. (est. 2013)
MIT License
100 stars 38 forks source link

Simplify event system #165

Open johnmaguire opened 4 years ago

johnmaguire commented 4 years ago

When I created the event system, I really just wanted a way for plugins to be able to subscribe to a data stream from other plugins, and ideally signal a message back to them. This was done for the URLs plugin so it could allow other plugins to override the URL title fetching behavior.

At the time, I wanted to provide some additional guarantees when loading a plugin that the event callback was invalid if the argspec didn't line up with the event emission. While this was admirable, I have actually noted that when using Cardinal, I tend to miss any error messages that occur during the initial plugin loading when Cardinal starts up to do all the noise in the logs. It may actually be beneficial to get an error in the logs every time that the callback is attempted to be called - it would certainly make the issue more obvious.

There are other downsides to the current approach as well - callbacks are unregistered when the event is unregistered, which results in bugs such as #143, and to a lesser extent, #142. Furthermore, the callback that ends up being registered isn't well-defined as far as its name is concerned. We refer to these with "callback IDs" which aren't very useful in the logs. If we'd like to configure events to be disable-able, we'll want them to be more addressable as ewll.

So instead, we'll make event callback subscription very lightweight - the event need not exist, and no validation is performed. Any plugin will then be able to send any amount of data via its channels.

johnmaguire commented 4 years ago

144 was another bug caused by this issue.

LucasFunai commented 2 years ago

Hello. I've found this page from the "good first issue" tag, and I do not have any experience in contributing to an open source project. I have decent understanding of Python, but I seem to always get lost when trying to understand the issues I'm trying to solve.

I'd like to contribute to fixing this bug, but I don't understand parts of the issue, like "argspec", and how the event callbacks work in general. What are some relevant source files that I should look for?

For now I will try to take a look on some of the files, but any help will be greatly appreciated.

LucasFunai commented 2 years ago

I've checked plugins.py and my understanding is that: Callbacks are invalid if it takes keyword arguments, or takes wrong amount of arguments, and raises EventCallbackError. The above error clogs the logs, making other bugs hard to fix. The ideal way to fix this is to fire errors when the problematic callback is attempted to be called, and not when it is registered, so the logging on register_callback(event_name,callback) in line 800 in plugins.py needs to go.

Is this correct?