Skip to content

fix: wrap float() in timeout_check with try/except for clean argparse error#2955

Open
imxde-code wants to merge 1 commit into
sherlock-project:masterfrom
imxde-code:fix/timeout-invalid-input-error
Open

fix: wrap float() in timeout_check with try/except for clean argparse error#2955
imxde-code wants to merge 1 commit into
sherlock-project:masterfrom
imxde-code:fix/timeout-invalid-input-error

Conversation

@imxde-code
Copy link
Copy Markdown

Summary

Fixes #2866

Passing a non-numeric value to --timeout (e.g. sherlock --timeout abc user) currently raises a raw ValueError from float(value) before argparse can intercept it, producing an ugly unformatted traceback.

Root cause

# Before — raw ValueError escapes argparse
def timeout_check(value):
    float_value = float(value)   # raises ValueError for non-numeric input
    if float_value <= 0:
        raise ArgumentTypeError(...)
    return float_value

Fix

# After — non-numeric input caught and re-raised as ArgumentTypeError
def timeout_check(value):
    try:
        float_value = float(value)
    except ValueError:
        raise ArgumentTypeError(
            f"Invalid timeout value: {value}. Timeout must be a positive number."
        )
    if float_value <= 0:
        raise ArgumentTypeError(
            f"Invalid timeout value: {value}. Timeout must be a positive number."
        )
    return float_value

Argparse then formats this consistently as:

error: argument --timeout: invalid timeout_check value: 'abc'

Tests added

Two parametrized regression tests in test_ux.py:

  • test_invalid_timeout_raises_argparse_error — covers non-numeric strings (abc, xyz, 1.2.3, None)
  • test_non_positive_timeout_raises_argparse_error — covers zero and negative values (pre-existing bug, now also tested)

Test plan

  • sherlock --timeout abc user → clean argparse error message
  • sherlock --timeout -1 user → clean argparse error message
  • sherlock --timeout 5 user → works normally
  • New tests pass

… error

Passing a non-numeric value to --timeout (e.g. 'abc') previously caused
a raw ValueError from float() to bubble up before argparse could handle
it, resulting in an ugly unformatted traceback instead of the standard
'argument --timeout: invalid' error message.

Wrapping the conversion in a try/except ValueError and re-raising as
ArgumentTypeError lets argparse format and surface the error correctly,
consistent with the existing behaviour for zero/negative values.

Also adds regression tests for both non-numeric and non-positive inputs.

Fixes sherlock-project#2866
@imxde-code imxde-code requested a review from ppfeister as a code owner May 14, 2026 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cli: invalid --timeout values raise a raw parser exception

1 participant