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 errors due to non-unpicklable Exception when "enqueue=True" #963

Merged
merged 1 commit into from
Aug 31, 2023

Conversation

Delgan
Copy link
Owner

@Delgan Delgan commented Aug 31, 2023

Fix #329.
Fix #504.

There are instances where a custom Exception might contain attributes that cannot be serialized or is incorrectly implemented (see Cannot unpickle Exception subclass). If this occurs with enqueue=True, an error will be raised by the background thread when it calls queue.get(). Due to the failure of de-serialization, the record is None, resulting in an inability to identify the origin of the error.

To address this concern, we can take a proactive step by attempting to unpickle the Exception beforehand. In the event of a failure, we replace its value with None to make sure it can be unpickled. This strategy mirrors what we already do for the serialization part. Actually, this approach was already implemented in the past. However, it was temporarily removed due to concerns raised about its safety (#563). Subsequent assessments have demonstrated its complete safety, allowing us to reintroduce it. Furthermore, we have expanded its scope to encompass all types of exceptions, not solely limited to UnpicklingError. This broader implementation serves to mitigate the aforementioned issues effectively.

@Delgan Delgan merged commit 565a6f6 into master Aug 31, 2023
18 checks passed
@Delgan Delgan deleted the GH-329-fix-non-unpicklable branch August 31, 2023 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant