Return None instead of raising OverflowError on an out-of-range value - #32
Open
eeshsaxena wants to merge 1 commit into
Open
Return None instead of raising OverflowError on an out-of-range value#32eeshsaxena wants to merge 1 commit into
eeshsaxena wants to merge 1 commit into
Conversation
timeparse already skips a malformed number field (like '1.2.3') and moves on to return None, but only ValueError was caught. A value large enough that float() overflows to inf makes int(sum(...)) raise OverflowError, which escaped and crashed the call. Catch OverflowError alongside ValueError so these behave like any other unparseable input.
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.
I was feeding some messy strings through
timeparseand hit a crash on a very large number:The value is syntactically fine, it is just enormous.
float()on it overflows toinf, and the integer-returning branch then doesint(sum(...)), which cannot convertinfand raisesOverflowError.The function already handles a malformed number field: a string like
'1.2.3 seconds'throwsValueErrorfromfloat(), which is caught so the parser moves on and ultimately returnsNone. The out-of-range case is the same situation, the field just is not a usable number, butOverflowErrorwas not in theexcept, so it escaped instead of falling through toNone.The fix is to catch
OverflowErroralongsideValueErrorin that block. After it,'9' * 400 + '.5 minutes'returnsNonelike any other input that cannot be parsed, and normal values are untouched.I added a test for the out-of-range case and the full suite still passes (50 tests).