Skip to content

fix: handle None query_time in xlsx and invalid timeout CLI args#2908

Open
mitre88 wants to merge 1 commit into
sherlock-project:masterfrom
mitre88:fix/xlsx-response-time-null
Open

fix: handle None query_time in xlsx and invalid timeout CLI args#2908
mitre88 wants to merge 1 commit into
sherlock-project:masterfrom
mitre88:fix/xlsx-response-time-null

Conversation

@mitre88
Copy link
Copy Markdown

@mitre88 mitre88 commented Apr 18, 2026

Summary

Changes

sherlock.py

  1. XLSX output fix (lines 921-925): Changed condition from if response_time_s is None (always false since it's a list) to if query_time is None where query_time = results[site]["status"].query_time

  2. Timeout validation (lines 533-538): Wrapped float(value) in try/except to catch ValueError and re-raise as ArgumentTypeError for consistent error handling

test_ux.py

Added test_invalid_timeout_values parametrized test covering:

  • --timeout 0 → "Invalid timeout value: 0"
  • --timeout -5 → "Invalid timeout value: -5"
  • --timeout abc → "Invalid timeout value: abc"

Testing

The new test can be run with: pytest tests/test_ux.py::test_invalid_timeout_values -v

Fixes #2891
Fixes #2866

- Fix xlsx output to use empty string when query_time is None instead
  of appending None (fixes sherlock-project#2891)
- Add try/except in timeout_check() to raise ArgumentTypeError for
  non-numeric timeout values (fixes sherlock-project#2866)
- Add regression tests for invalid timeout values
@mitre88 mitre88 requested a review from ppfeister as a code owner April 18, 2026 06:02
@mitre88
Copy link
Copy Markdown
Author

mitre88 commented Apr 21, 2026

Hi maintainers! I've addressed the issues mentioned in the issues:

  • Fixed xlsx output to handle None query_time properly
  • Added validation for invalid timeout values (0, -5, non-numeric)
  • Added regression tests

Would love to get feedback on this PR

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.

Bug: xlsx output always appends None for response_time_s when query_time is missing cli: invalid --timeout values raise a raw parser exception

1 participant