utils: extract classify_http_error helper, refactor 3 callsites (#38)#39
Merged
Conversation
The same urllib HTTPError + URLError + OSError + TimeoutError dispatch repeats in three sites after #15/#18/#20: plugins/devagentic-canvas/client.py:_request plugins/devagentic-docs/client.py:_post_graphql hermes_cli/doctor.py:_check_devagentic_graph Each site has the same if-401/403-elif-404-else branches over the exception and the same urllib network-error fallback. Only the message text differs per site. Add `utils.classify_http_error(exc) -> str` returning one of: HTTP_ERROR_AUTH — 401 / 403 HTTP_ERROR_NOT_FOUND — 404 HTTP_ERROR_HTTP — other HTTPError status HTTP_ERROR_UNREACHABLE — URLError / OSError / TimeoutError HTTP_ERROR_UNKNOWN — anything else Refactor the three callsites onto a single `except (URLError, OSError, TimeoutError)` (HTTPError is a URLError subclass) followed by a dispatch on the classifier output. Each site still owns its own message text — the user-facing strings are unchanged. Behavior change: none. Net diff is roughly even (helper + dispatch replaces three near-identical if/elif/else blocks). Tests: - 11 new tests in tests/test_utils_classify_http_error.py covering 401, 403, 404, generic statuses (400/422/429/500/502/503/504), URLError, OSError, TimeoutError, socket.timeout, unrelated exceptions, and the HTTPError-is-URLError subclass quirk. - All three refactored callsites' existing test suites still pass: canvas (47), docs (47), doctor devagentic-graph (6) — 112 total. Closes #38.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #38. The urllib HTTPError + URLError + OSError + TimeoutError dispatch repeats verbatim in three sites after #15/#18/#20:
plugins/devagentic-canvas/client.py:_requestplugins/devagentic-docs/client.py:_post_graphqlhermes_cli/doctor.py:_check_devagentic_graphEach does the same 401/403 → auth, 404 → not_found, other → http, URLError/OSError/TimeoutError → unreachable dispatch. Only the user-facing message text differs.
Fix
Add
utils.classify_http_error(exc) -> strreturning one of:HTTP_ERROR_AUTHHTTP_ERROR_NOT_FOUNDHTTP_ERROR_HTTPHTTP_ERROR_UNREACHABLEHTTP_ERROR_UNKNOWNRefactor the three callsites onto a single
except (URLError, OSError, TimeoutError)block (HTTPError is a URLError subclass) followed by a dispatch on the classifier output. Each site keeps its own message text — user-facing strings are byte-for-byte unchanged.Behavior change
None. Same exception coverage, same message text, same paths through the code.
Test plan
tests/test_utils_classify_http_error.py:HTTP_ERROR_HTTPURLError,OSError,TimeoutError,socket.timeout→ unreachableValueError,RuntimeError("401 Unauthorized")→HTTP_ERROR_UNKNOWN(string-match heuristics don't apply here — they belong in callsite-specific code like the Mem0 probe)pytest tests/test_utils_classify_http_error.py tests/test_devagentic_{canvas,docs}_plugin.py tests/hermes_cli/test_doctor_devagentic_graph.py→ 112 passed (11 new + 47 canvas + 47 docs + 6 doctor; verifies the refactored callsites still produce identical output).Why not the Mem0 message-substring classifier?
Mem0 (#34/#35) uses string-substring matching on
str(exc)because the mem0ai SDK doesn't surfaceHTTPError— it wraps responses into custom exceptions. That's a different taxonomy. Currently only one site uses that pattern; not yet 3 sites. Will extract if/when another SDK uses the same shape.Filed by hermes-maintainer (PowerCreek).