bot icon indicating copy to clipboard operation
bot copied to clipboard

Potential Unhandled Exceptions in Perpetual Tasks

Open MarkKoz opened this issue 6 years ago • 7 comments

The bot often makes use of the pattern of loop.create_task with tasks that are meant to run perpetually. However, there are no provisions for restarting tasks should any exceptions occur in them.

Discord.py's tasks extension looks promising and that could be used going forward. As an alternative, some wrapper could be created that just logs exceptions and tries again, or using Future.add_done_callback. There are probably more possible approaches.

See discussion in #360 for details on a particular case.

MarkKoz avatar May 07 '19 00:05 MarkKoz

The tasks extension is definitely going to be beneficial, however it's not on stable release yet, only due for usage in v1.1 when it comes out (current in dev testing, installable via git only). There's likely to be a fair bit of breaking changes to review when that comes out also, along with more subtle feature changes that we may want to properly review when it actually drops, but I'd be keen to see it added at that time too.

I was going to recommend the add_done_callback method also, as it's what I currently use, and I believe I've used it in the past for seasonalbot.

scragly avatar May 07 '19 00:05 scragly

Maybe it'll be released by the time we're done moving over to Django.

MarkKoz avatar May 07 '19 00:05 MarkKoz

v1.1 is now released.

scragly avatar May 14 '19 04:05 scragly

v1.6.0 is now released. 🎉

On a more serious note, 360 has been merged for over a year, tasks should probably be pretty stable, and this issue has been dormant for so long it probably has not come up at all since. Have we stopped using create_task? I don't believe so.

The library support is there for it, but the question is: is this still relevant?

HassanAbouelela avatar Feb 27 '21 14:02 HassanAbouelela

Yes. It may seem like it doesn't come up because exceptions are not caught anywhere and this is more of a proactive measure than a reactive one.

I made some progress by implementing this https://github.com/python-discord/bot/blob/master/bot/utils/scheduling.py#L160-L164

However, there is no support for scheduling on a loop (which is necessary to be able to schedule tasks before the loop starts) nor is there support for retrying upon failure. Still, logging exceptions is beneficial for a start.

MarkKoz avatar Feb 27 '21 18:02 MarkKoz

I may be able to implement this.

onerandomusername avatar Dec 13 '21 01:12 onerandomusername

I'd suggest you hold off for now. Scheduling is one of the first things being ported to bot-core, and I expect a PR within the next couple days.

HassanAbouelela avatar Dec 13 '21 11:12 HassanAbouelela

For creating tasks we now use pydis_core.utils.scheduling.create_task , which handles creating tasks.

However, we don't use it for perpetual tasks any more, so I don't think retrying is that important for most uses, and in some cases could cause more problems that it solves. Ideally retrying logic should be as precise as possible (e.g. retrying an individual request rather than a whole task) to avoid repeating operations that shouldn't be repeated.

For perpetual tasks we now use discord.ext.tasks.loop, which isn't perfect, but there's an issue to improve error reporting/retrying logic for it on the bot-core repo: https://github.com/python-discord/bot-core/issues/186.

I'll close this as I don't think there's anything actionable on this repo right now.

wookie184 avatar Jan 31 '24 14:01 wookie184