No exception on job timeout

[kiriusm2]

In v0.20 when job is timed out asyncio.TimeoutError is not raised. Instead there is log message: job_name cancelled, will be run again and job is restarted immediately.
Is it intentional? Having some exception on timeout and allow app to decide how to handle it seems more appropriate to me.

[samuelcolvin]

I think asyncio.TimeoutError is raised, it's just caught and the job is re-run.

If you want to do something different, you might be able to catch it inside the job?

[kiriusm2]

Inside the job there is asyncio.CancelledError on timeout now.
I guess it's because in #212
async with async_timeout.timeout(timeout_s):
was replaced with
cancel_handler = self.loop.call_at(self.loop.time() + timeout_s, task.cancel)

[samuelcolvin]

Humm, the the logic I implemented was copied from async-timeout, so I would have expected the behaviour to be the same.

Guess I'm wrong...

[kiriusm2]

https://github.com/aio-libs/async-timeout/blob/master/async_timeout/__init__.py#L191-L194
async-timeout checks exception type in __exit__
and raises asyncio.TimeoutError if it's asyncio.CancelledError and task was cancelled by async-timeout.
I can't find similar logic in arq

[samuelcolvin]

Oh, missed that. Happy to accept a PR if we can fix it.

[kiriusm2]

Is it ok to just return async-timeout back or there is some deep reason behind removing it?

[samuelcolvin]

yes, we need access to the task to add it to self.job_tasks

[samuelcolvin]

For stuff like this, it's worth reading the code rather than asking questions, it's relatively clear in the code:

https://github.com/samuelcolvin/arq/blob/8ee04f96934684c9b8f8278df0530ae9fad1be4a/arq/worker.py#L520-L535

[kiriusm2]

Actually inside a job it's always been asyncio.CancelledError on timeout. The difference only in retries. In 0.20 job is retried and previously exception was just logged.

[samuelcolvin]

Humm, I think we should revert this to just log the error, you can always catch the CancelledError and raise a retry exception.

Would you create a PR?

[JonasKs]

This seems to have been resolved.