Skip to content

Raise the task's exception from imap() iterators - #480

Open
LuisFigueroaG wants to merge 1 commit into
celery:mainfrom
LuisFigueroaG:fix/imap-raise-task-exception
Open

LuisFigueroaG wants to merge 1 commit into
celery:mainfrom
LuisFigueroaG:fix/imap-raise-task-exception

Conversation

@LuisFigueroaG

Copy link
Copy Markdown
Contributor

When a task fails, IMapIterator.next() raises Exception(value), where value is the ExceptionInfo, instead of the exception the task raised. Code iterating over imap()/imap_unordered() can't catch the error by its type (except ValueError never matches) and only gets a bare Exception wrapping the ExceptionInfo. apply_async().get() and map() already raise the original exception.

This raises value.exception, the same as ApplyResult.get().

With a task that raises ValueError('bad item 1'):

apply_async().get(): ValueError('bad item 1')
map():               ValueError('bad item 1')
imap() before:       Exception(<ExceptionInfo: ValueError('bad item 1')>)
imap() after:        ValueError('bad item 1')

Added a unit test for imap() and imap_unordered().

When a task failed, IMapIterator.next() raised Exception(ExceptionInfo)
instead of the exception the task raised, so code iterating over
imap()/imap_unordered() could not catch it by type ('except ValueError'
never matched) and only got a bare Exception whose message was the
remote traceback. apply_async().get() and map() raise the original
exception.

Raise value.exception, as ApplyResult.get() does. The worker's
traceback is still attached as the exception's __cause__.

@auvipy auvipy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this a follow up of your previous PR?

@LuisFigueroaG

Copy link
Copy Markdown
Contributor Author

Not a follow-up, they're independent. #475 keeps the Supervisor from crashing when a worker exits while an imap() is still pending (the iterator had no _write_to/_scheduled_for). This one is about what IMapIterator.next() raises when a task fails: it wrapped the ExceptionInfo in a plain Exception instead of raising the task's own exception like ApplyResult.get() does. I found both while looking at imap(), but either can go in first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants