Skip to content

Wrap connection and TLS errors from RemoteClient as NetworkError - #324

Open
pcbeingused333 wants to merge 1 commit into
pyfenn:mainfrom
pcbeingused333:fix/remote-client-wrap-connection-errors
Open

pcbeingused333 wants to merge 1 commit into
pyfenn:mainfrom
pcbeingused333:fix/remote-client-wrap-connection-errors

Conversation

@pcbeingused333

Copy link
Copy Markdown
Contributor

Problem

RemoteClient._request wraps requests.exceptions.Timeout as NetworkError, but re-raises ConnectionError and SSLError unchanged:

except requests.exceptions.SSLError:
    raise
except requests.exceptions.ConnectionError:
    raise
except requests.exceptions.Timeout as exc:
    raise NetworkError(...) from exc

The two except X: raise clauses are no-ops. fenn.exceptions.NetworkError is documented as "The remote host could not be reached", and the CLI only handles the typed errors:

  • fenn runexcept RemoteError (NetworkError is a subclass)
  • fenn authexcept (NetworkError, RemoteError)

So a DNS failure, refused connection, or bad TLS certificate when talking to pyfenn.com escapes as a raw requests traceback instead of the intended Remote error: ... message and clean exit.

Fix

Wrap ConnectionError and SSLError in NetworkError too, keeping distinct wording (Could not connect to ... / TLS error connecting to ...) and chaining the original with from exc. SSLError stays a separate clause (it's a ConnectionError subclass) so its message is kept.

Tests

tests/unit/remote/test_client.py — the first tests for RemoteClient: transport errors (connection / SSL / connect-timeout / read-timeout) all raise NetworkError; a 200 still returns; 401/402 still map to AuthError / InsufficientCreditsError. The connection/SSL/connect-timeout cases raise raw requests exceptions on main.

Verified: pytest tests/unit — 924 passed; ruff check/format and ty check fenn/remote/client.py clean. (The pre-commit CI job is red on a pre-existing ty failure in fenn/experimental/vision/normalize.py and fenn/logging.py, unrelated to this change.)

_request wrapped requests.Timeout as NetworkError but re-raised
ConnectionError and SSLError unchanged (the 'except X: raise' clauses
were no-ops). fenn.exceptions.NetworkError is documented as 'the remote
host could not be reached', and the CLI (fenn run, fenn auth) only
catches NetworkError / RemoteError -- so a DNS failure, refused
connection or bad TLS cert to pyfenn.com surfaced as an unhandled
requests traceback instead of a clean 'Remote error' message.

Wrap both in NetworkError (keeping distinct 'Could not connect' /
'TLS error' wording), chaining the original with 'from exc'. Adds
tests/unit/remote/test_client.py -- the first tests for RemoteClient.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018uqJtTJWawYLVA5EdpUmho
@pcbeingused333

Copy link
Copy Markdown
Contributor Author

The pre-commit check is red on the ty hook, pre-existing on main (commit 0d1862e fails the same job on fenn/experimental/vision/normalize.py and fenn/logging.py, neither touched here). ruff check / ruff format and ty check fenn/remote/client.py are clean locally; the test jobs are the relevant ones.

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