errno_from_exception: require int in args fallback#3661
Open
HrachShah wants to merge 1 commit into
Open
Conversation
… args don't silently bypass the comparison errno_from_exception has two branches: it returns the exception's errno attribute when present, otherwise it falls back to args[0]. Every internal caller compares the result against an int errno constant (e.g. errno_from_exception(e) == errno.EBADF), so the args fallback has to return an int. The pre-fix code returned args[0] as-is, which meant a str message (the shape used by most third-party OSError subclasses, by OSError subclasses that only set a human-readable message, and by any value like ValueError or RuntimeError raised with a string) flowed into the int-comparison call sites and silently turned every comparison into False. The real error class (and the right recovery branch) was hidden. Pin the fallback to return args[0] only when it is an int, otherwise None -- which matches the call sites' expectations and surfaces the 'unknown errno' case the same way as the empty-args path. Add six tests in tornado/test/util_test.py:ErrnoFromExceptionTest covering the errno attribute winning over args, an int args[0] being returned, an empty args tuple returning None (the function used to raise TypeError), a non-int args[0] returning None, an int args[1] not being picked up when args[0] is a message, and a message-only exception returning None.
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.
Summary
errno_from_exceptionreturns the exception'serrnoattribute when present, otherwise it falls back toargs[0]. Every internal caller compares the result against an int errno constant (e.g.errno_from_exception(e) == errno.EBADF), so the args fallback has to return an int.The pre-fix code returned
args[0]as-is, which meant astrmessage — the shape used by most third-partyOSErrorsubclasses, byOSErrorsubclasses that only set a human-readable message, and by anyValueError/RuntimeErrorraised with a string — flowed into the int-comparison call sites and silently turned every comparison intoFalse. The real error class (and the right recovery branch) was hidden.Fix
Pin the fallback to return
args[0]only when it is anint, otherwiseNone— which matches the call sites' expectations and surfaces the "unknown errno" case the same way as the empty-args path.Tests
Six new tests in
tornado/test/util_test.py:ErrnoFromExceptionTest:test_errno_attr_wins_over_argstest_args0_int_returned_when_no_errno_attrtest_empty_args_returns_none(the function used to raiseTypeErrorhere)test_non_int_args0_returns_nonetest_non_int_args0_with_int_args1_returns_nonetest_message_only_args_returns_noneAll 6 pass. The 6 new tests fail on the pre-fix code (with
TypeErrorfor the empty-args case and the wrong return value for the rest) and pass with the fix.