Handle raising exceptions from PublicTask #4078
Conversation
When the task raises an exception there is no way to have a custom result under ``AsyncResult.info``, it will be always the Exception that was risen from inside the task. Because of that, when the task raises an Exception we are handling it inside the ``run`` method and we add the exception's message into our custom result *without marking* the task as FAILURE.
|
As the description says, this is a workaround. Today I was pointed to use I need to read a little more about that since I'm not familiar. |
|
That worked, but I think that we still need a way to be able to access the I opened this issue upstream looking for help: celery/kombu#868 |
|
I got a reply on the upstream issue that I opened, but I tested and I wasn't able to make it work. I'd like to merge this PR since it fixes the issue and make it work as it was before and take a deeper look to a better solution later in another PR instead of blocking finding the best solution here. |
| if STATUS_UPDATES_ENABLED: | ||
| self.update_state(state=status, meta=info) | ||
| if error and exception_raised: | ||
| info['error'] = str(exception_raised) |
There was a problem hiding this comment.
Is there a reason we are passing the string here instead of the actual Exception object? It seems like this limits what we can do with it later on
There was a problem hiding this comment.
Yes. I had different problems when trying to serialize an Exception object. In fact, this serialization problem was one of the things that originated this issue.
| if error is not None and isinstance(error, Exception): | ||
| data['error'] = error.message | ||
| if error is not None: | ||
| data['error'] = error |
There was a problem hiding this comment.
Similarly, we should probably we defensive here about what error is. Are we sure it will always be a string type, or should we be actively turning it into a string here, also?
There was a problem hiding this comment.
It should be always a string since that data object here is the info dictionary that we are setting in the run method of the task where we cast the Exception object to a string.
Mhm, I'm not sure. I added two different tests. One that tests the result returned when a PublicTask fails and another one that tests the It should be good to improve our test coverage on these functionality in another PR, also. |
* Handle raising exceptions from PublicTask When the task raises an exception there is no way to have a custom result under ``AsyncResult.info``, it will be always the Exception that was risen from inside the task. Because of that, when the task raises an Exception we are handling it inside the ``run`` method and we add the exception's message into our custom result *without marking* the task as FAILURE. * Test case for a PublicTask that raises an Exception * Test case for method used to check a job status
When the task raises an exception there is no way to have a custom result under
AsyncResult.info, it will be always the Exception that was risen from inside the task.Because of that, when the task raises an Exception we are handling it inside the
runmethod and we add the exception's message into our custom result without marking the task as FAILURE.Celery docs: http://docs.celeryproject.org/en/latest/reference/celery.result.html#celery.result.AsyncResult.info
Workaround to #4077