Skip to content

narrow version-check exception handler to specific request and JSON errors#3006

Open
HrachShah wants to merge 1 commit into
sherlock-project:masterfrom
HrachShah:fix/sherlock-version-check-narrow-exception
Open

narrow version-check exception handler to specific request and JSON errors#3006
HrachShah wants to merge 1 commit into
sherlock-project:masterfrom
HrachShah:fix/sherlock-version-check-narrow-exception

Conversation

@HrachShah

Copy link
Copy Markdown

The bare except Exception in the post-argparse version-check guarded against only three concrete failure modes:

  • requests.get(forge_api_latest_release, timeout=10) raises requests.RequestException (and its concrete subclasses ConnectionError, Timeout, HTTPError) for network and transport failures.
  • json_loads(latest_release_raw) raises json.JSONDecodeError (a ValueError subclass) for malformed JSON.
  • latest_release_json['tag_name'] and latest_remote_tag[1:] raise KeyError / IndexError for unexpected payload shapes.

Catching Exception was masking unrelated bugs and KeyboardInterrupt (e.g. a Ctrl-C between the signal.signal call above and the try block). The narrower tuple keeps the original 'silently report the version-check failure' behaviour for the three real failure modes and lets other exceptions propagate with their full traceback.

Tested locally: python3 -c "import ast; ast.parse(...)" parses; the version-check still prints 'A problem occurred while checking for an update' for the network/JSON failure modes it was originally guarding against.

…or, KeyError)

The bare `except Exception` in the post-argparse version-check only guarded
against three concrete failure modes:

  - requests.get(forge_api_latest_release, timeout=10) raises
    requests.RequestException (and its concrete subclasses ConnectionError,
    Timeout, HTTPError) for network and transport failures.
  - json_loads(latest_release_raw) raises json.JSONDecodeError (a
    ValueError subclass) for malformed JSON, and int() conversion of
    tag_name substrings raises ValueError on bad digits.
  - latest_release_json['tag_name'] and latest_remote_tag[1:] raise
    KeyError / IndexError for unexpected payload shapes.

Catching Exception was masking unrelated bugs (a typo in
forge_api_latest_release, an unhandled unicode issue in latest_release_raw,
a broken Response.json shortcut) and turning them into a confusing
'A problem occurred while checking for an update' line on stderr, and
KeyboardInterrupt / SystemExit raised during a Ctrl-C just after the
signal handler install (between the signal.signal line above and the
try/except block) would also be swallowed here, leaving the user with a
half-shut-down process. The narrower tuple keeps the original 'silently
report the version-check failure' behaviour for the three real failure
modes and lets other exceptions propagate with their full traceback.
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.

1 participant