Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix Processor.process exception handling #650

Merged
merged 2 commits into from
Mar 10, 2022

Conversation

bfis
Copy link
Contributor

@bfis bfis commented Mar 10, 2022

Fixes the exception handling behavior for the Processor.process call by correctly chaining exceptions.

Reintroduces the benefits of #360.
Reintroduces the behavior of #370, since this is working as intended in python's concurrent/futures.
Retrains the fix for #438.
Fixes #649.

@nsmith-
Copy link
Member

nsmith- commented Mar 10, 2022

So the key is to add a new exception to the chain rather than attempt to modify the last exception's message?
We're approaching java levels of traceback length sadly, but it is too hazardous to try to edit them I think.

@andrzejnovak
Copy link
Collaborator

Filtering the traceback for a duplicate substring over some length would be too fragile?

@bfis
Copy link
Contributor Author

bfis commented Mar 10, 2022

We're approaching java levels of traceback length sadly, [...].

Yes, surprisingly, there is no tooling/help from python's side with handling/catching exceptions that are down the chain - it seems to me the original PEP-3134 did not concern itself too much with the handling of exception chains it introduced.

Filtering the traceback for a duplicate substring over some length would be too fragile?

Messing around with exception in such a away is a recipe for disaster since they have no fixed format and can do just about anything (e.g. tracebacks can be formatted very differently etc.). The underlying "duplication"/formatting issue lies in the internals of python`s concurrent.futures.ProcessPoolExecutor, so should be remedied there.

@lgray lgray merged commit 5dc2470 into CoffeaTeam:master Mar 10, 2022
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.

[Bug] Exception raising processor_instance.process(events) fails for custom Exception signatures
4 participants