Shouldn't _has_failed variable in discord.ext.tasks.Loop not be reset after a coro exception ?

[nmarotte]

Hi everyone,

As I was implementing a dynamic command loop for my Discord Bot, I had to look inside the Loop class and I notice something strange. Here is the method in particular

    async def _loop(self, *args, **kwargs):
        backoff = ExponentialBackoff()
        await self._call_loop_function('before_loop')
        sleep_until = discord.utils.sleep_until
        self._last_iteration_failed = False
        self._next_iteration = datetime.datetime.now(datetime.timezone.utc)
        try:
            await asyncio.sleep(0) # allows canceling in before_loop
            while True:
                if not self._last_iteration_failed:
                    self._last_iteration = self._next_iteration
                    self._next_iteration = self._get_next_sleep_time()
                try:
                    await self.coro(*args, **kwargs)
                    self._last_iteration_failed = False
                    now = datetime.datetime.now(datetime.timezone.utc)
                    if now > self._next_iteration:
                        self._next_iteration = now
                except self._valid_exception as exc:
                    self._last_iteration_failed = True
                    if not self.reconnect:
                        raise
                    await asyncio.sleep(backoff.delay())
                else:
                    if self._stop_next_iteration:
                        return
                    self._current_loop += 1
                    if self._current_loop == self.count:
                        break

                    await sleep_until(self._next_iteration)
        except asyncio.CancelledError:
            self._is_being_cancelled = True
            raise
        except Exception as exc:
            self._has_failed = True
            await self._call_loop_function('error', exc)
            raise exc
        finally:
            await self._call_loop_function('after_loop')
            self._is_being_cancelled = False
            self._current_loop = 0
            self._stop_next_iteration = False
            self._has_failed = False

My problem with that is in the last part, handling exception. As we can see, if there is almost any exception, it will be caught with the line except Exception as exc: and the variable self._has_failed will be set to True, however in the finally part, that is always executed, whether exceptions are to be raised or not, we immediately set it back to False.

So my question/discussions is : is this behavior intended ? It seems to me that the variable _has_failed is next to useless since it is always reset almost immediately

I see the sense in that, since we should have it back to False to loop again, but the problem is that I think we will never be able to see in any code that the variable _has_failed is True and its sole use is to know if the coro had an exception or not

I'm writing this because for my loop command, I want to know if the given command to be executed every X second failed or not but _has_failed always returns False even if the coro raised an exception (and only once, since its execution seems stopped after raising the exception)

I'm not sure if this is an issue or maybe a miscomprehension on my part so that's why I'm posting it here and not in issues

[Rapptz]

You're misreading the code.

The outer finally (outside of the while True) resets the state of the loop class so it can be reused.