From 95fd3511f504adbe4ac806b4c706ff106466e4fb Mon Sep 17 00:00:00 2001
From: MarkKoz <KozlovMark@gmail.com>
Date: Sat, 15 Feb 2020 19:57:58 -0800
Subject: Scheduler: fix #754 - only suppress CancelledError

---
 bot/utils/scheduling.py | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/bot/utils/scheduling.py b/bot/utils/scheduling.py
index ee6c0a8e6..8d4721d70 100644
--- a/bot/utils/scheduling.py
+++ b/bot/utils/scheduling.py
@@ -63,12 +63,13 @@ def create_task(loop: asyncio.AbstractEventLoop, coro_or_future: Union[Coroutine
     """Creates an asyncio.Task object from a coroutine or future object."""
     task: asyncio.Task = asyncio.ensure_future(coro_or_future, loop=loop)
 
-    # Silently ignore exceptions in a callback (handles the CancelledError nonsense)
-    task.add_done_callback(_silent_exception)
+    # Silently ignore CancelledError in a callback
+    task.add_done_callback(_suppress_cancelled_error)
     return task
 
 
-def _silent_exception(future: asyncio.Future) -> None:
-    """Suppress future's exception."""
-    with contextlib.suppress(Exception):
-        future.exception()
+def _suppress_cancelled_error(future: asyncio.Future) -> None:
+    """Suppress future's CancelledError exception."""
+    if future.cancelled():
+        with contextlib.suppress(asyncio.CancelledError):
+            future.exception()
-- 
cgit v1.2.3


From f905f73451730fb5b83b441f8d32748acef374e0 Mon Sep 17 00:00:00 2001
From: MarkKoz <KozlovMark@gmail.com>
Date: Sat, 15 Feb 2020 20:07:26 -0800
Subject: Scheduler: remove create_task function

It's redundant because the done callback only takes a single line to add
and can be added in schedule_task().

* Use Task as the type hint rather than Future for
  _suppress_cancelled_error()
---
 bot/utils/scheduling.py | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/bot/utils/scheduling.py b/bot/utils/scheduling.py
index 8d4721d70..7b055f5e7 100644
--- a/bot/utils/scheduling.py
+++ b/bot/utils/scheduling.py
@@ -2,7 +2,7 @@ import asyncio
 import contextlib
 import logging
 from abc import abstractmethod
-from typing import Coroutine, Dict, Union
+from typing import Dict
 
 from bot.utils import CogABCMeta
 
@@ -41,7 +41,8 @@ class Scheduler(metaclass=CogABCMeta):
             )
             return
 
-        task: asyncio.Task = create_task(loop, self._scheduled_task(task_data))
+        task = loop.create_task(self._scheduled_task(task_data))
+        task.add_done_callback(_suppress_cancelled_error)
 
         self.scheduled_tasks[task_id] = task
         log.debug(f"{self.cog_name}: scheduled task #{task_id}.")
@@ -59,17 +60,8 @@ class Scheduler(metaclass=CogABCMeta):
         del self.scheduled_tasks[task_id]
 
 
-def create_task(loop: asyncio.AbstractEventLoop, coro_or_future: Union[Coroutine, asyncio.Future]) -> asyncio.Task:
-    """Creates an asyncio.Task object from a coroutine or future object."""
-    task: asyncio.Task = asyncio.ensure_future(coro_or_future, loop=loop)
-
-    # Silently ignore CancelledError in a callback
-    task.add_done_callback(_suppress_cancelled_error)
-    return task
-
-
-def _suppress_cancelled_error(future: asyncio.Future) -> None:
-    """Suppress future's CancelledError exception."""
-    if future.cancelled():
+def _suppress_cancelled_error(task: asyncio.Task) -> None:
+    """Suppress a task's CancelledError exception."""
+    if task.cancelled():
         with contextlib.suppress(asyncio.CancelledError):
-            future.exception()
+            task.exception()
-- 
cgit v1.2.3


From f5cd7e357de26c81b462b8935ec4bdaa032429fc Mon Sep 17 00:00:00 2001
From: MarkKoz <KozlovMark@gmail.com>
Date: Sat, 15 Feb 2020 20:10:42 -0800
Subject: Scheduler: correct schedule_task's docstring

---
 bot/utils/scheduling.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bot/utils/scheduling.py b/bot/utils/scheduling.py
index 7b055f5e7..adf10d683 100644
--- a/bot/utils/scheduling.py
+++ b/bot/utils/scheduling.py
@@ -33,7 +33,7 @@ class Scheduler(metaclass=CogABCMeta):
         """
         Schedules a task.
 
-        `task_data` is passed to `Scheduler._scheduled_expiration`
+        `task_data` is passed to the `Scheduler._scheduled_task()` coroutine.
         """
         if task_id in self.scheduled_tasks:
             log.debug(
-- 
cgit v1.2.3


From 0d05be37564b1ec8babd688fb348c2c13eeb9fa2 Mon Sep 17 00:00:00 2001
From: MarkKoz <KozlovMark@gmail.com>
Date: Sat, 15 Feb 2020 20:16:30 -0800
Subject: Scheduler: remove loop parameter from schedule_task

asyncio.create_task() exists and will already use the running loop in
the current thread. Because there is no intention of using a different
loop in a different thread anywhere in the program for the foreseeable
future, the loop parameter is redundant.
---
 bot/cogs/moderation/management.py   |  4 +---
 bot/cogs/moderation/scheduler.py    |  4 ++--
 bot/cogs/moderation/superstarify.py |  2 +-
 bot/cogs/reminders.py               | 11 +++--------
 bot/utils/scheduling.py             |  4 ++--
 5 files changed, 9 insertions(+), 16 deletions(-)

diff --git a/bot/cogs/moderation/management.py b/bot/cogs/moderation/management.py
index f2964cd78..279c8b809 100644
--- a/bot/cogs/moderation/management.py
+++ b/bot/cogs/moderation/management.py
@@ -1,4 +1,3 @@
-import asyncio
 import logging
 import textwrap
 import typing as t
@@ -133,8 +132,7 @@ class ModManagement(commands.Cog):
 
             # If the infraction was not marked as permanent, schedule a new expiration task
             if request_data['expires_at']:
-                loop = asyncio.get_event_loop()
-                self.infractions_cog.schedule_task(loop, new_infraction['id'], new_infraction)
+                self.infractions_cog.schedule_task(new_infraction['id'], new_infraction)
 
             log_text += f"""
                 Previous expiry: {old_infraction['expires_at'] or "Permanent"}
diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py
index e14c302cb..62b040d1f 100644
--- a/bot/cogs/moderation/scheduler.py
+++ b/bot/cogs/moderation/scheduler.py
@@ -48,7 +48,7 @@ class InfractionScheduler(Scheduler):
         )
         for infraction in infractions:
             if infraction["expires_at"] is not None and infraction["type"] in supported_infractions:
-                self.schedule_task(self.bot.loop, infraction["id"], infraction)
+                self.schedule_task(infraction["id"], infraction)
 
     async def reapply_infraction(
         self,
@@ -150,7 +150,7 @@ class InfractionScheduler(Scheduler):
                 await action_coro
                 if expiry:
                     # Schedule the expiration of the infraction.
-                    self.schedule_task(ctx.bot.loop, infraction["id"], infraction)
+                    self.schedule_task(infraction["id"], infraction)
             except discord.HTTPException as e:
                 # Accordingly display that applying the infraction failed.
                 confirm_msg = f":x: failed to apply"
diff --git a/bot/cogs/moderation/superstarify.py b/bot/cogs/moderation/superstarify.py
index 050c847ac..d94ee6891 100644
--- a/bot/cogs/moderation/superstarify.py
+++ b/bot/cogs/moderation/superstarify.py
@@ -145,7 +145,7 @@ class Superstarify(InfractionScheduler, Cog):
         log.debug(f"Changing nickname of {member} to {forced_nick}.")
         self.mod_log.ignore(constants.Event.member_update, member.id)
         await member.edit(nick=forced_nick, reason=reason)
-        self.schedule_task(ctx.bot.loop, id_, infraction)
+        self.schedule_task(id_, infraction)
 
         # Send a DM to the user to notify them of their new infraction.
         await utils.notify_infraction(
diff --git a/bot/cogs/reminders.py b/bot/cogs/reminders.py
index 45bf9a8f4..d96dedd20 100644
--- a/bot/cogs/reminders.py
+++ b/bot/cogs/reminders.py
@@ -1,4 +1,3 @@
-import asyncio
 import logging
 import random
 import textwrap
@@ -42,7 +41,6 @@ class Reminders(Scheduler, Cog):
         )
 
         now = datetime.utcnow()
-        loop = asyncio.get_event_loop()
 
         for reminder in response:
             remind_at = datetime.fromisoformat(reminder['expiration'][:-1])
@@ -53,7 +51,7 @@ class Reminders(Scheduler, Cog):
                 await self.send_reminder(reminder, late)
 
             else:
-                self.schedule_task(loop, reminder["id"], reminder)
+                self.schedule_task(reminder["id"], reminder)
 
     @staticmethod
     async def _send_confirmation(ctx: Context, on_success: str) -> None:
@@ -88,10 +86,8 @@ class Reminders(Scheduler, Cog):
 
     async def _reschedule_reminder(self, reminder: dict) -> None:
         """Reschedule a reminder object."""
-        loop = asyncio.get_event_loop()
-
         self.cancel_task(reminder["id"])
-        self.schedule_task(loop, reminder["id"], reminder)
+        self.schedule_task(reminder["id"], reminder)
 
     async def send_reminder(self, reminder: dict, late: relativedelta = None) -> None:
         """Send the reminder."""
@@ -185,8 +181,7 @@ class Reminders(Scheduler, Cog):
             on_success=f"Your reminder will arrive in {humanize_delta(relativedelta(expiration, now))}!"
         )
 
-        loop = asyncio.get_event_loop()
-        self.schedule_task(loop, reminder["id"], reminder)
+        self.schedule_task(reminder["id"], reminder)
 
     @remind_group.command(name="list")
     async def list_reminders(self, ctx: Context) -> Optional[Message]:
diff --git a/bot/utils/scheduling.py b/bot/utils/scheduling.py
index adf10d683..a16900066 100644
--- a/bot/utils/scheduling.py
+++ b/bot/utils/scheduling.py
@@ -29,7 +29,7 @@ class Scheduler(metaclass=CogABCMeta):
         then make a site API request to delete the reminder from the database.
         """
 
-    def schedule_task(self, loop: asyncio.AbstractEventLoop, task_id: str, task_data: dict) -> None:
+    def schedule_task(self, task_id: str, task_data: dict) -> None:
         """
         Schedules a task.
 
@@ -41,7 +41,7 @@ class Scheduler(metaclass=CogABCMeta):
             )
             return
 
-        task = loop.create_task(self._scheduled_task(task_data))
+        task = asyncio.create_task(self._scheduled_task(task_data))
         task.add_done_callback(_suppress_cancelled_error)
 
         self.scheduled_tasks[task_id] = task
-- 
cgit v1.2.3


From 6b7c0a7a74460ee96c5ce574bf042f3de38dd685 Mon Sep 17 00:00:00 2001
From: MarkKoz <KozlovMark@gmail.com>
Date: Sat, 15 Feb 2020 20:24:48 -0800
Subject: Scheduler: raise task exceptions besides CancelledError

Explicitly retrieves the task's exception, which will raise the
exception if one exists.

* Rename _suppress_cancelled_error to _handle_task_exception
---
 bot/utils/scheduling.py | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/bot/utils/scheduling.py b/bot/utils/scheduling.py
index a16900066..df46ccdd9 100644
--- a/bot/utils/scheduling.py
+++ b/bot/utils/scheduling.py
@@ -13,8 +13,9 @@ class Scheduler(metaclass=CogABCMeta):
     """Task scheduler."""
 
     def __init__(self):
+        # Keep track of the child cog's name so the logs are clear.
+        self.cog_name = self.__class__.__name__
 
-        self.cog_name = self.__class__.__name__  # keep track of the child cog's name so the logs are clear.
         self.scheduled_tasks: Dict[str, asyncio.Task] = {}
 
     @abstractmethod
@@ -42,7 +43,7 @@ class Scheduler(metaclass=CogABCMeta):
             return
 
         task = asyncio.create_task(self._scheduled_task(task_data))
-        task.add_done_callback(_suppress_cancelled_error)
+        task.add_done_callback(_handle_task_exception)
 
         self.scheduled_tasks[task_id] = task
         log.debug(f"{self.cog_name}: scheduled task #{task_id}.")
@@ -60,8 +61,10 @@ class Scheduler(metaclass=CogABCMeta):
         del self.scheduled_tasks[task_id]
 
 
-def _suppress_cancelled_error(task: asyncio.Task) -> None:
-    """Suppress a task's CancelledError exception."""
+def _handle_task_exception(task: asyncio.Task) -> None:
+    """Raise the task's exception, if any, unless the task is cancelled and has a CancelledError."""
     if task.cancelled():
         with contextlib.suppress(asyncio.CancelledError):
             task.exception()
+    else:
+        task.exception()
-- 
cgit v1.2.3


From 687b6404e8976ffdc67ba9492a4355819f06a2f7 Mon Sep 17 00:00:00 2001
From: MarkKoz <KozlovMark@gmail.com>
Date: Sat, 15 Feb 2020 21:17:22 -0800
Subject: Scheduler: cancel the task in the callback

This design makes more sense and is more convenient than requiring tasks
to be responsible for cancelling themselves.

* Rename _handle_task_exception to _task_done_callback
* Add trace logging
---
 bot/cogs/reminders.py   |  6 +++---
 bot/utils/scheduling.py | 36 +++++++++++++++++++++++++++---------
 2 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/bot/cogs/reminders.py b/bot/cogs/reminders.py
index d96dedd20..603b627fb 100644
--- a/bot/cogs/reminders.py
+++ b/bot/cogs/reminders.py
@@ -74,9 +74,6 @@ class Reminders(Scheduler, Cog):
         log.debug(f"Deleting reminder {reminder_id} (the user has been reminded).")
         await self._delete_reminder(reminder_id)
 
-        # Now we can begone with it from our schedule list.
-        self.cancel_task(reminder_id)
-
     async def _delete_reminder(self, reminder_id: str) -> None:
         """Delete a reminder from the database, given its ID, and cancel the running task."""
         await self.bot.api_client.delete('bot/reminders/' + str(reminder_id))
@@ -86,7 +83,10 @@ class Reminders(Scheduler, Cog):
 
     async def _reschedule_reminder(self, reminder: dict) -> None:
         """Reschedule a reminder object."""
+        log.trace(f"Cancelling old task #{reminder['id']}")
         self.cancel_task(reminder["id"])
+
+        log.trace(f"Scheduling new task #{reminder['id']}")
         self.schedule_task(reminder["id"], reminder)
 
     async def send_reminder(self, reminder: dict, late: relativedelta = None) -> None:
diff --git a/bot/utils/scheduling.py b/bot/utils/scheduling.py
index df46ccdd9..40d26249f 100644
--- a/bot/utils/scheduling.py
+++ b/bot/utils/scheduling.py
@@ -2,6 +2,7 @@ import asyncio
 import contextlib
 import logging
 from abc import abstractmethod
+from functools import partial
 from typing import Dict
 
 from bot.utils import CogABCMeta
@@ -36,6 +37,8 @@ class Scheduler(metaclass=CogABCMeta):
 
         `task_data` is passed to the `Scheduler._scheduled_task()` coroutine.
         """
+        log.trace(f"{self.cog_name}: scheduling task #{task_id}...")
+
         if task_id in self.scheduled_tasks:
             log.debug(
                 f"{self.cog_name}: did not schedule task #{task_id}; task was already scheduled."
@@ -43,13 +46,15 @@ class Scheduler(metaclass=CogABCMeta):
             return
 
         task = asyncio.create_task(self._scheduled_task(task_data))
-        task.add_done_callback(_handle_task_exception)
+        task.add_done_callback(partial(self._task_done_callback, task_id))
 
         self.scheduled_tasks[task_id] = task
-        log.debug(f"{self.cog_name}: scheduled task #{task_id}.")
+        log.debug(f"{self.cog_name}: scheduled task #{task_id} {id(task)}.")
 
     def cancel_task(self, task_id: str) -> None:
         """Un-schedules a task."""
+        log.trace(f"{self.cog_name}: cancelling task #{task_id}...")
+
         task = self.scheduled_tasks.get(task_id)
 
         if task is None:
@@ -57,14 +62,27 @@ class Scheduler(metaclass=CogABCMeta):
             return
 
         task.cancel()
-        log.debug(f"{self.cog_name}: unscheduled task #{task_id}.")
+        log.debug(f"{self.cog_name}: unscheduled task #{task_id} {id(task)}.")
         del self.scheduled_tasks[task_id]
 
+    def _task_done_callback(self, task_id: str, task: asyncio.Task) -> None:
+        """
+        Unschedule the task and raise its exception if one exists.
+
+        If the task was cancelled, the CancelledError is retrieved and suppressed. In this case,
+        the task is already assumed to have been unscheduled.
+        """
+        log.trace(f"{self.cog_name}: performing done callback for task #{task_id} {id(task)}")
+
+        if task.cancelled():
+            with contextlib.suppress(asyncio.CancelledError):
+                task.exception()
+        else:
+            # Check if it exists to avoid logging a warning.
+            if task_id in self.scheduled_tasks:
+                # Only cancel if the task is not cancelled to avoid a race condition when a new
+                # task is scheduled using the same ID. Reminders do this when re-scheduling after
+                # editing.
+                self.cancel_task(task_id)
 
-def _handle_task_exception(task: asyncio.Task) -> None:
-    """Raise the task's exception, if any, unless the task is cancelled and has a CancelledError."""
-    if task.cancelled():
-        with contextlib.suppress(asyncio.CancelledError):
             task.exception()
-    else:
-        task.exception()
-- 
cgit v1.2.3


From 6edae6cda82add4d0bf538e916ce571432f83ab1 Mon Sep 17 00:00:00 2001
From: MarkKoz <KozlovMark@gmail.com>
Date: Sat, 15 Feb 2020 21:21:51 -0800
Subject: Moderation: avoid prematurely cancelling deactivation task

Because deactivate_infraction() explicitly cancels the scheduled task,
it now runs in a separate task to avoid prematurely cancelling itself.
---
 bot/cogs/moderation/scheduler.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py
index 62b040d1f..7d401c7ff 100644
--- a/bot/cogs/moderation/scheduler.py
+++ b/bot/cogs/moderation/scheduler.py
@@ -415,4 +415,6 @@ class InfractionScheduler(Scheduler):
         expiry = dateutil.parser.isoparse(infraction["expires_at"]).replace(tzinfo=None)
         await time.wait_until(expiry)
 
-        await self.deactivate_infraction(infraction)
+        # Because deactivate_infraction() explicitly cancels this scheduled task, it runs in
+        # a separate task to avoid prematurely cancelling itself.
+        self.bot.loop.create_task(self.deactivate_infraction(infraction))
-- 
cgit v1.2.3


From f09d6b0646edb62806b60b145986b7cc680fd77c Mon Sep 17 00:00:00 2001
From: MarkKoz <KozlovMark@gmail.com>
Date: Sat, 15 Feb 2020 21:34:38 -0800
Subject: Scheduler: make _scheduled_tasks private

Main concern is someone trying to cancel a task directly. The workaround
for the race condition relies on the task only being cancelled via
Scheduler.cancel_task(), particularly because it removes the task from
the dictionary. The done callback will not remove from the dictionary
if it sees the task has already been cancelled. So it's a bad idea to
cancel tasks directly...
---
 bot/utils/scheduling.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/bot/utils/scheduling.py b/bot/utils/scheduling.py
index 40d26249f..0d66952eb 100644
--- a/bot/utils/scheduling.py
+++ b/bot/utils/scheduling.py
@@ -17,7 +17,7 @@ class Scheduler(metaclass=CogABCMeta):
         # Keep track of the child cog's name so the logs are clear.
         self.cog_name = self.__class__.__name__
 
-        self.scheduled_tasks: Dict[str, asyncio.Task] = {}
+        self._scheduled_tasks: Dict[str, asyncio.Task] = {}
 
     @abstractmethod
     async def _scheduled_task(self, task_object: dict) -> None:
@@ -39,7 +39,7 @@ class Scheduler(metaclass=CogABCMeta):
         """
         log.trace(f"{self.cog_name}: scheduling task #{task_id}...")
 
-        if task_id in self.scheduled_tasks:
+        if task_id in self._scheduled_tasks:
             log.debug(
                 f"{self.cog_name}: did not schedule task #{task_id}; task was already scheduled."
             )
@@ -48,14 +48,14 @@ class Scheduler(metaclass=CogABCMeta):
         task = asyncio.create_task(self._scheduled_task(task_data))
         task.add_done_callback(partial(self._task_done_callback, task_id))
 
-        self.scheduled_tasks[task_id] = task
+        self._scheduled_tasks[task_id] = task
         log.debug(f"{self.cog_name}: scheduled task #{task_id} {id(task)}.")
 
     def cancel_task(self, task_id: str) -> None:
         """Un-schedules a task."""
         log.trace(f"{self.cog_name}: cancelling task #{task_id}...")
 
-        task = self.scheduled_tasks.get(task_id)
+        task = self._scheduled_tasks.get(task_id)
 
         if task is None:
             log.warning(f"{self.cog_name}: Failed to unschedule {task_id} (no task found).")
@@ -63,7 +63,7 @@ class Scheduler(metaclass=CogABCMeta):
 
         task.cancel()
         log.debug(f"{self.cog_name}: unscheduled task #{task_id} {id(task)}.")
-        del self.scheduled_tasks[task_id]
+        del self._scheduled_tasks[task_id]
 
     def _task_done_callback(self, task_id: str, task: asyncio.Task) -> None:
         """
@@ -79,7 +79,7 @@ class Scheduler(metaclass=CogABCMeta):
                 task.exception()
         else:
             # Check if it exists to avoid logging a warning.
-            if task_id in self.scheduled_tasks:
+            if task_id in self._scheduled_tasks:
                 # Only cancel if the task is not cancelled to avoid a race condition when a new
                 # task is scheduled using the same ID. Reminders do this when re-scheduling after
                 # editing.
-- 
cgit v1.2.3


From df87aba432db50eb480ba8b2f42b1a64147909d9 Mon Sep 17 00:00:00 2001
From: MarkKoz <KozlovMark@gmail.com>
Date: Tue, 25 Feb 2020 20:15:58 -0800
Subject: Moderation: use asyncio.shield to prevent self-cancellation

The shield exists to be used for exactly this purpose so its a better
fit than create_task.
---
 bot/cogs/moderation/scheduler.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py
index 162159af8..93afd9f9f 100644
--- a/bot/cogs/moderation/scheduler.py
+++ b/bot/cogs/moderation/scheduler.py
@@ -1,3 +1,4 @@
+import asyncio
 import logging
 import textwrap
 import typing as t
@@ -427,6 +428,6 @@ class InfractionScheduler(Scheduler):
         expiry = dateutil.parser.isoparse(infraction["expires_at"]).replace(tzinfo=None)
         await time.wait_until(expiry)
 
-        # Because deactivate_infraction() explicitly cancels this scheduled task, it runs in
-        # a separate task to avoid prematurely cancelling itself.
-        self.bot.loop.create_task(self.deactivate_infraction(infraction))
+        # Because deactivate_infraction() explicitly cancels this scheduled task, it is shielded
+        # to avoid prematurely cancelling itself.
+        await asyncio.shield(self.deactivate_infraction(infraction))
-- 
cgit v1.2.3


From b1f8f4779738be35e1339d6c07e317ef08009467 Mon Sep 17 00:00:00 2001
From: MarkKoz <KozlovMark@gmail.com>
Date: Tue, 25 Feb 2020 21:20:20 -0800
Subject: Scheduler: improve cancel_task's docstring

* Use imperative mood for docstring
* Explain the purpose of the parameter in the docstring
* Make log message after cog name lowercase
---
 bot/utils/scheduling.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/bot/utils/scheduling.py b/bot/utils/scheduling.py
index 0d66952eb..cb28648db 100644
--- a/bot/utils/scheduling.py
+++ b/bot/utils/scheduling.py
@@ -52,19 +52,19 @@ class Scheduler(metaclass=CogABCMeta):
         log.debug(f"{self.cog_name}: scheduled task #{task_id} {id(task)}.")
 
     def cancel_task(self, task_id: str) -> None:
-        """Un-schedules a task."""
+        """Unschedule the task identified by `task_id`."""
         log.trace(f"{self.cog_name}: cancelling task #{task_id}...")
-
         task = self._scheduled_tasks.get(task_id)
 
-        if task is None:
-            log.warning(f"{self.cog_name}: Failed to unschedule {task_id} (no task found).")
+        if not task:
+            log.warning(f"{self.cog_name}: failed to unschedule {task_id} (no task found).")
             return
 
         task.cancel()
-        log.debug(f"{self.cog_name}: unscheduled task #{task_id} {id(task)}.")
         del self._scheduled_tasks[task_id]
 
+        log.debug(f"{self.cog_name}: unscheduled task #{task_id} {id(task)}.")
+
     def _task_done_callback(self, task_id: str, task: asyncio.Task) -> None:
         """
         Unschedule the task and raise its exception if one exists.
-- 
cgit v1.2.3


From 4a11bf22cc9e894271b896eb9fca0c3cff085766 Mon Sep 17 00:00:00 2001
From: MarkKoz <KozlovMark@gmail.com>
Date: Tue, 25 Feb 2020 22:22:01 -0800
Subject: Scheduler: only delete the task in the done callback if tasks are
 same

To prevent a deletion of task rescheduled with the same ID, the callback
checks that the stored task is the same as the done task being handled.

* Only delete the task; it doesn't need to be cancelled because the it
  is already done
* Revise the callback's docstring to explain the new behaviour
* Rename `task` parameter to `done_task`
---
 bot/utils/scheduling.py | 41 +++++++++++++++++++++++++----------------
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/bot/utils/scheduling.py b/bot/utils/scheduling.py
index cb28648db..58bb32e5d 100644
--- a/bot/utils/scheduling.py
+++ b/bot/utils/scheduling.py
@@ -65,24 +65,33 @@ class Scheduler(metaclass=CogABCMeta):
 
         log.debug(f"{self.cog_name}: unscheduled task #{task_id} {id(task)}.")
 
-    def _task_done_callback(self, task_id: str, task: asyncio.Task) -> None:
+    def _task_done_callback(self, task_id: str, done_task: asyncio.Task) -> None:
         """
-        Unschedule the task and raise its exception if one exists.
+        Delete the task and raise its exception if one exists.
 
-        If the task was cancelled, the CancelledError is retrieved and suppressed. In this case,
-        the task is already assumed to have been unscheduled.
+        If `done_task` and the task associated with `task_id` are different, then the latter
+        will not be deleted. In this case, a new task was likely rescheduled with the same ID.
         """
-        log.trace(f"{self.cog_name}: performing done callback for task #{task_id} {id(task)}")
+        log.trace(f"{self.cog_name}: performing done callback for task #{task_id} {id(done_task)}.")
 
-        if task.cancelled():
-            with contextlib.suppress(asyncio.CancelledError):
-                task.exception()
+        scheduled_task = self._scheduled_tasks.get(task_id)
+
+        if scheduled_task and done_task is scheduled_task:
+            # A task for the ID exists and its the same as the done task.
+            # Since this is the done callback, the task is already done so no need to cancel it.
+            log.trace(f"{self.cog_name}: deleting task #{task_id} {id(done_task)}.")
+            del self._scheduled_tasks[task_id]
+        elif scheduled_task:
+            # A new task was likely rescheduled with the same ID.
+            log.debug(
+                f"{self.cog_name}: the scheduled task #{task_id} {id(scheduled_task)} "
+                f"and the done task {id(done_task)} differ."
+            )
         else:
-            # Check if it exists to avoid logging a warning.
-            if task_id in self._scheduled_tasks:
-                # Only cancel if the task is not cancelled to avoid a race condition when a new
-                # task is scheduled using the same ID. Reminders do this when re-scheduling after
-                # editing.
-                self.cancel_task(task_id)
-
-            task.exception()
+            log.warning(
+                f"{self.cog_name}: task #{task_id} not found while handling task {id(done_task)}! "
+                f"A task somehow got unscheduled improperly (i.e. deleted but not cancelled)."
+            )
+
+        with contextlib.suppress(asyncio.CancelledError):
+            done_task.exception()
-- 
cgit v1.2.3


From 47b645a2cd2622709c57158d788554544579d870 Mon Sep 17 00:00:00 2001
From: MarkKoz <KozlovMark@gmail.com>
Date: Tue, 25 Feb 2020 22:23:07 -0800
Subject: Scheduler: properly raise task's exception the done callback

Task.exception() only returns the exception. It still needs to be
explicitly raised.
---
 bot/utils/scheduling.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/bot/utils/scheduling.py b/bot/utils/scheduling.py
index 58bb32e5d..742133f02 100644
--- a/bot/utils/scheduling.py
+++ b/bot/utils/scheduling.py
@@ -94,4 +94,7 @@ class Scheduler(metaclass=CogABCMeta):
             )
 
         with contextlib.suppress(asyncio.CancelledError):
-            done_task.exception()
+            exception = done_task.exception()
+            # Raise the exception if one exists.
+            if exception:
+                raise exception
-- 
cgit v1.2.3


From e173cd2af20b546230ed467f26286ee167df55cd Mon Sep 17 00:00:00 2001
From: MarkKoz <KozlovMark@gmail.com>
Date: Tue, 25 Feb 2020 22:34:35 -0800
Subject: Scheduler: only send warning in callback if task isn't cancelled

If a task is cancelled it is assumed it was done via cancel_task. That
method deletes the task after cancelling so the warning isn't relevant.
---
 bot/utils/scheduling.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bot/utils/scheduling.py b/bot/utils/scheduling.py
index 742133f02..9371dcdb7 100644
--- a/bot/utils/scheduling.py
+++ b/bot/utils/scheduling.py
@@ -87,7 +87,7 @@ class Scheduler(metaclass=CogABCMeta):
                 f"{self.cog_name}: the scheduled task #{task_id} {id(scheduled_task)} "
                 f"and the done task {id(done_task)} differ."
             )
-        else:
+        elif not done_task.cancelled():
             log.warning(
                 f"{self.cog_name}: task #{task_id} not found while handling task {id(done_task)}! "
                 f"A task somehow got unscheduled improperly (i.e. deleted but not cancelled)."
-- 
cgit v1.2.3


From 4096ef526aba41ab3fd83be16ef3b5554419d524 Mon Sep 17 00:00:00 2001
From: MarkKoz <KozlovMark@gmail.com>
Date: Thu, 27 Feb 2020 19:43:45 -0800
Subject: Scheduler: log the exception instead of raising

Logging it ourselves has a cleaner traceback and gives more control
over the output, such as including the task ID.
---
 bot/utils/scheduling.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/bot/utils/scheduling.py b/bot/utils/scheduling.py
index 9371dcdb7..1eae817c1 100644
--- a/bot/utils/scheduling.py
+++ b/bot/utils/scheduling.py
@@ -95,6 +95,9 @@ class Scheduler(metaclass=CogABCMeta):
 
         with contextlib.suppress(asyncio.CancelledError):
             exception = done_task.exception()
-            # Raise the exception if one exists.
+            # Log the exception if one exists.
             if exception:
-                raise exception
+                log.error(
+                    f"{self.cog_name}: error in task #{task_id} {id(scheduled_task)}!",
+                    exc_info=exception
+                )
-- 
cgit v1.2.3


From 5daf1db8ea9e86568da4907d42507aa3286eb3c1 Mon Sep 17 00:00:00 2001
From: MarkKoz <KozlovMark@gmail.com>
Date: Fri, 28 Feb 2020 09:59:57 -0800
Subject: Scheduler: correct type annotations

---
 bot/utils/scheduling.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/bot/utils/scheduling.py b/bot/utils/scheduling.py
index 1eae817c1..5760ec2d4 100644
--- a/bot/utils/scheduling.py
+++ b/bot/utils/scheduling.py
@@ -1,9 +1,9 @@
 import asyncio
 import contextlib
 import logging
+import typing as t
 from abc import abstractmethod
 from functools import partial
-from typing import Dict
 
 from bot.utils import CogABCMeta
 
@@ -17,10 +17,10 @@ class Scheduler(metaclass=CogABCMeta):
         # Keep track of the child cog's name so the logs are clear.
         self.cog_name = self.__class__.__name__
 
-        self._scheduled_tasks: Dict[str, asyncio.Task] = {}
+        self._scheduled_tasks: t.Dict[t.Hashable, asyncio.Task] = {}
 
     @abstractmethod
-    async def _scheduled_task(self, task_object: dict) -> None:
+    async def _scheduled_task(self, task_object: t.Any) -> None:
         """
         A coroutine which handles the scheduling.
 
@@ -31,7 +31,7 @@ class Scheduler(metaclass=CogABCMeta):
         then make a site API request to delete the reminder from the database.
         """
 
-    def schedule_task(self, task_id: str, task_data: dict) -> None:
+    def schedule_task(self, task_id: t.Hashable, task_data: t.Any) -> None:
         """
         Schedules a task.
 
@@ -51,7 +51,7 @@ class Scheduler(metaclass=CogABCMeta):
         self._scheduled_tasks[task_id] = task
         log.debug(f"{self.cog_name}: scheduled task #{task_id} {id(task)}.")
 
-    def cancel_task(self, task_id: str) -> None:
+    def cancel_task(self, task_id: t.Hashable) -> None:
         """Unschedule the task identified by `task_id`."""
         log.trace(f"{self.cog_name}: cancelling task #{task_id}...")
         task = self._scheduled_tasks.get(task_id)
@@ -65,7 +65,7 @@ class Scheduler(metaclass=CogABCMeta):
 
         log.debug(f"{self.cog_name}: unscheduled task #{task_id} {id(task)}.")
 
-    def _task_done_callback(self, task_id: str, done_task: asyncio.Task) -> None:
+    def _task_done_callback(self, task_id: t.Hashable, done_task: asyncio.Task) -> None:
         """
         Delete the task and raise its exception if one exists.
 
-- 
cgit v1.2.3