Skip to content

fix(networking): catch real exceptions in get_external_ip fallback chain#3318

Open
ArtificialXai wants to merge 2 commits intolatent-to:stagingfrom
ArtificialXai:fix/artificialxai/networking-ip-fallback-chain
Open

fix(networking): catch real exceptions in get_external_ip fallback chain#3318
ArtificialXai wants to merge 2 commits intolatent-to:stagingfrom
ArtificialXai:fix/artificialxai/networking-ip-fallback-chain

Conversation

@ArtificialXai
Copy link
Copy Markdown

Summary

Closes #3309.

Each provider in get_external_ip is wrapped in except ExternalIPNotFound, but none of the underlying calls (requests.get, ip_to_int, os.popen + readline, urllib.request.urlopen, json.loads, the Wikipedia header lookup) ever raise ExternalIPNotFound. As a result the first failing provider crashes the entire function instead of falling through to the next one — exactly the symptom reported in the issue (a requests.exceptions.ConnectionError propagates out when AWS is unreachable, rather than the curl fallback being attempted).

Fix

Replace each except ExternalIPNotFound: with a tuple of exceptions that the providers actually raise:

Exception Where it originates
requests.exceptions.RequestException requests.get(...) (AWS, Wikipedia)
netaddr.core.AddrFormatError ip_to_int(...) with malformed input
OSError os.popen(...) + readline() (ifconfig.me, ipinfo.io, dnsomatic)
urllib.error.URLError urllib_request.urlopen(...) (ident.me)
ValueError json.loads(...) parse failures, int(...) parse failures
KeyError Wikipedia response missing X-Client-IP header
AssertionError assert isinstance(ip_to_int(...), int) guard
ExternalIPNotFound Kept for backward compatibility with any wrapper that raises it

The set is defined once at module scope as _IP_LOOKUP_EXCEPTIONS so the providers stay in sync.

Tests

Three new regression tests in tests/unit_tests/utils/test_networking.py:

  • test_get_external_ip_aws_connection_error_falls_through_to_curl — AWS requests.get raises ConnectionError, curl fallback returns a valid IP, function returns that IP (was: raised ConnectionError).
  • test_get_external_ip_malformed_ip_falls_through — AWS returns a non-IP string (triggers AddrFormatError in ip_to_int), curl fallback succeeds.
  • test_get_external_ip_all_providers_exhausted_raises — every provider raises its real exception; function must raise ExternalIPNotFound (not leak the last underlying exception).

All 22 tests in tests/unit_tests/utils/test_networking.py pass locally.

Test plan

  • pytest tests/unit_tests/utils/test_networking.py — all 22 pass
  • CI run on PR

Notes

  • The existing assert isinstance(ip_to_int(external_ip), int) line is tautological (ip_to_int either returns int or raises), but left unchanged to keep this PR narrow and focused on the reported bug.
  • No behavior change when every provider succeeds — only the error-path taxonomy is corrected.

Copy link
Copy Markdown
Collaborator

@basfroman basfroman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @ArtificialXai, pls read CONTRIBUTING.md to move forward with your pr.

@ArtificialXai ArtificialXai force-pushed the fix/artificialxai/networking-ip-fallback-chain branch from 3dea36d to 7f31808 Compare April 20, 2026 21:13
@ArtificialXai ArtificialXai changed the base branch from master to staging April 20, 2026 21:13
@ArtificialXai
Copy link
Copy Markdown
Author

Thanks for the pointer, @basfroman. Read CONTRIBUTING.md and updated the PR:

  • Rebased onto staging and changed the PR base (was master).
  • Rewrote the commit message to match the 6-rule style (subject <50 chars, body wrapped at 72, explains what and why).
  • PR is still <50 files (2 files, 96/+6/-); the 3 regression tests in tests/unit_tests/utils/test_networking.py cover the real exception paths.

Ready for another look when you have a minute.

@thewhaleking
Copy link
Copy Markdown
Contributor

Thanks for the pointer, @basfroman. Read CONTRIBUTING.md and updated the PR:

  • Rebased onto staging and changed the PR base (was master).
  • Rewrote the commit message to match the 6-rule style (subject <50 chars, body wrapped at 72, explains what and why).
  • PR is still <50 files (2 files, 96/+6/-); the 3 regression tests in tests/unit_tests/utils/test_networking.py cover the real exception paths.

Ready for another look when you have a minute.

Your commit still isn't signed

Closes latent-to#3309.

Each provider in `get_external_ip` was wrapped in
`except ExternalIPNotFound`, but none of the underlying calls
(`requests.get`, `ip_to_int`, `os.popen`+`readline`,
`urllib.request.urlopen`, `json.loads`, wikipedia header lookup)
ever raise `ExternalIPNotFound`. As a result the first failing
provider crashed the entire function instead of falling through
to the next one.

Catch the exceptions that can actually occur:
  * RequestException  — for `requests.get` failures
  * AddrFormatError   — for malformed IPs
  * OSError           — for `os.popen`/curl failures
  * URLError          — for urllib failures
  * ValueError        — for json + int parse failures
  * KeyError          — for the wikipedia header
  * AssertionError    — for the isinstance guards
  * ExternalIPNotFound — kept for backward compat

Three regression tests cover: AWS connection error → curl
fallthrough, malformed AWS IP → curl fallthrough, and
all-providers-exhausted → ExternalIPNotFound.
@ArtificialXai ArtificialXai force-pushed the fix/artificialxai/networking-ip-fallback-chain branch from 7f31808 to d361f28 Compare April 24, 2026 15:08
@ArtificialXai
Copy link
Copy Markdown
Author

Signed and re-pushed (d361f287).

@thewhaleking thewhaleking requested a review from basfroman April 24, 2026 15:10
@ArtificialXai
Copy link
Copy Markdown
Author

Hey @basfroman, friendly ping — addressed all your feedback in the 2026-04-20 comment (rebased onto staging, commit message follows 6-rule style, tests cover the exceptions). Commit is now signed as of today (2026-04-24). Could you take another look when you get a moment? Thanks!

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.

get_external_ip fallback chain never triggers — all attempts catch ExternalIPNotFound but none of the called functions raise it

3 participants