Repository navigation
gh-156942: Raise the exception where the marshalling error is detected - #156944
Conversation
…etected
Previously the marshal writer recorded an error code and converted it into
an exception at the end, replacing the exception which was already raised
with ValueError("unmarshallable object"). Error messages now name the type
of the unsupported object and the required version.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
This started as pure refactoring and error message improvement, but several minor errors was found. |
|
@vstinner, could you please look also at this PR? |
vstinner
left a comment
vstinner
left a comment
There was a problem hiding this comment.
LGTM. All error code paths raise an exception and tests check the error message.
I'm not sure that these two constants are still useful:
// Error codes:
#define WFERR_OK 0
#define WFERR_EXCEPTION_SET 1 /* An exception has been raised. */
You might make a following change to just check "wf.error". If it's zero, there is no error. If it's non-zero, an exception is set.
# Conflicts: # Lib/test/test_marshal.py
|
I left them to minimize the diff, but if you prefer, this can be done on one PR. |
If you want to remove WFERR_OK and WFERR_EXCEPTION_SET, IMO it's better to do that in a separated PR. I agree that it's better to have small changes. |
|
I already applied this here. Thank you for your review. |
2 participants
The writer now raises the exception where the error is detected, instead of recording an error code and converting it into an exception at the end.
WFILE.errorbecomes a flag saying that an exception has been raised,w_set_exception()and all error codes except one are removed.The exception which was already raised is no longer replaced:
BufferErrorfor a non-contiguous buffer,TypeErrorfor uncomparable set items,MemoryErrorfor a failed allocation, and the exception raised while marshalling a set item are propagated as they are.Messages which were
ValueError("unmarshallable object")now name the type of the object and, if the type is only supported by newer data formats, the required version.The error paths were mostly untested, so the tests for them are added, and the existing tests now check the error message.