Don't leak TypeError from Coerce when the type is a non-class callable#549
Open
chuenchen309 wants to merge 1 commit into
Open
Don't leak TypeError from Coerce when the type is a non-class callable#549chuenchen309 wants to merge 1 commit into
chuenchen309 wants to merge 1 commit into
Conversation
Coerce accepts any callable (`type: Union[type, Callable]`), and its
docstring promises that a ValueError/TypeError from the constructor marks
the value Invalid. But the failure handler called
`issubclass(self.type, Enum)`, which raises `TypeError: issubclass() arg 1
must be a class` when `self.type` is a function -- and that call is outside
the try, so it escaped as a raw TypeError instead of a CoerceInvalid.
So `Schema(Coerce(datetime.fromisoformat))('x')` or any
`Coerce(some_parse_func)` crashed on the invalid-input path rather than
producing MultipleInvalid, bypassing error paths and `humanize`.
Guard the Enum check with `isinstance(self.type, type)`; the class path
(including the Enum value listing) is unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: chuenchen309 <48723787+chuenchen309@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Coerceaccepts any callable (type: Union[type, Callable]), and its docstring says aValueError/TypeErrorfrom the constructor marks the valueInvalid. But when the callable is a plain function (not a class), the failure path raises a raw, uncaughtTypeErrorinstead:Coerce(int)('foo')(a class) correctly yieldsMultipleInvalid: expected int. The only difference is class-vs-function, so the two are analogous — and functions are explicitly supported by the signature.Cause
In
Coerce.__call__, theexcepthandler callsissubclass(self.type, Enum)to optionally list Enum values in the message.issubclass()requires a class as its first argument, so whenself.typeis a function it raisesTypeError. That call is outside thetry(which only wrapsself.type(v)), so it propagates raw, bypassingCoerceInvalid, error paths, andhumanize.Note
Coerce(func, msg="...")didn't hit this, becausenot self.msgshort-circuits beforeissubclass— only the default-message path was affected.Fix
Guard the Enum check with
isinstance(self.type, type):The class path (including the Enum value-listing message, e.g.
expected Color or one of 1, 2) is unchanged; non-class callables now produceMultipleInvalid: expected <name>as documented.Testing
Added
test_coerce_callable_marks_invalid_without_msg(red before, green after). Full suite: 182 passed.black,isort,flake8,mypyall clean.Disclosure: authored by an AI coding agent (Claude Code) running on this account — it found the bug, ran the repro, wrote the test and this description. I review every change and am accountable for it; the verification above is real and re-runnable from the diff. If this isn't a change you want, say so and I'll close it.