fix(tianditu): add request timeouts and credential guards#3469
Open
Harsh23Kashyap wants to merge 2 commits into
Open
fix(tianditu): add request timeouts and credential guards#3469Harsh23Kashyap wants to merge 2 commits into
Harsh23Kashyap wants to merge 2 commits into
Conversation
Mirrors the same three-bug reliability pattern from PR langgenius#3456 (tools/google), and the matching PRs langgenius#3466 (tools/searchapi) and langgenius#3468 (tools/serper): - requests.get had no timeout= argument in any of the three tool files (geocoder.py, staticmap.py, poisearch.py). Stalled hosts blocked until the framework-level 120s ceiling. Add timeout=10 to every call site. - self.runtime.credentials["tianditu_api_key"] raised KeyError when the key was absent or empty. Replace with an early guard that yields a friendly error message and returns; use .get() for the value lookup. - Wrap the HTTP calls in try/except requests.RequestException so a timeout, connection reset, DNS failure, or 5xx surfaces as a friendly message instead of a stack trace. tianditu does not take user-supplied hl/gl (the geocoder API uses a keyWord and the user's region comes from upstream), so the invoke fall-through bug from PR langgenius#3456 does not apply. No public API change; successful 200-path responses are byte-identical. Adds tools/tianditu/tests/test_tianditu.py with 11 in-process test cases covering all three tool files. Bumps manifest 0.0.7 -> 0.0.8. Refactors nothing else; the staticmap.PoiSearchTool class name is kept as-is to avoid unrelated churn. (It is mis-named — the file is the static-map image tool, not POI search — but renaming is out of scope.) Refs langgenius#3465 Refs langgenius#3456
dosubot
Bot
added
size:L
Contributor
There was a problem hiding this comment.
Pull request overview
Adds reliability safeguards to Tianditu tools.
Changes:
- Adds credential guards, 10-second request timeouts, and network-error handling.
- Adds in-process regression tests.
- Bumps plugin version to
0.0.8.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
tools/tianditu/tools/geocoder.py |
Hardens geocoding requests. |
tools/tianditu/tools/staticmap.py |
Hardens static-map requests. |
tools/tianditu/tools/poisearch.py |
Hardens POI requests. |
tools/tianditu/tests/test_tianditu.py |
Adds regression coverage. |
tools/tianditu/tests/__init__.py |
Marks the test package. |
tools/tianditu/manifest.yaml |
Bumps the patch version. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+28
to
+31
| result = requests.get( | ||
| base_url + "?ds=" + json.dumps(params, ensure_ascii=False) + "&tk=" + tk, | ||
| timeout=10, | ||
| ).json() |
Comment on lines
+32
to
+35
| except requests.exceptions.RequestException as exc: | ||
| yield self.create_text_message( | ||
| f"An error occurred while invoking the tool: {exc}." | ||
| ) |
Comment on lines
+51
to
+54
| except requests.exceptions.RequestException as exc: | ||
| yield self.create_text_message( | ||
| f"An error occurred while invoking the tool: {exc}." | ||
| ) |
Comment on lines
+58
to
+61
| except requests.exceptions.RequestException as exc: | ||
| yield self.create_text_message( | ||
| f"An error occurred while invoking the tool: {exc}." | ||
| ) |
Comment on lines
+32
to
+35
| + "&tk=" | ||
| + tk, | ||
| timeout=10, | ||
| ).json() |
Comment on lines
+49
to
+50
| timeout=10, | ||
| ).content |
Comment on lines
+36
to
+39
| + "&tk=" | ||
| + tk, | ||
| timeout=10, | ||
| ).json() |
Comment on lines
+55
to
+57
| + tk, | ||
| timeout=10, | ||
| ).json() |
…sages Copilot review flagged two issues on PR langgenius#3469: - requests.get skipped raise_for_status(), so 4xx/5xx were treated as success. - str(exc) in the yielded message can include the request URL with tk. Add raise_for_status() before parsing each response. Use a generic network/upstream failure message that never forwards URLs or credentials. Refs langgenius#3465
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
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
Mirrors the same three-bug reliability pattern that PR #3456 (already MERGED on 2026-07-20,
tools/google) and PR #3466 (tools/searchapi) / PR #3468 (tools/serper) just fixed, applied totools/tianditu.Refs #3465 (umbrella tracker).
Changes
requests.get(...)call across the three tool files (geocoder.py,staticmap.py,poisearch.py) now passestimeout=10. 10 s is well above the upstream Tianditu API p99 and well below the framework-level 120 s ceiling.self.runtime.credentials["tianditu_api_key"]is replaced with an early guard that yields"Tianditu API key is required."and returns; the value is read via.get(). No moreKeyErroron a missing or empty key.try/except requests.exceptions.RequestExceptionso a timeout, connection reset, DNS failure, or 5xx surfaces as a friendly message instead of a stack trace.tools/tianditu/tests/test_tianditu.pywith 11 in-process test cases (nopytestordify_pluginruntime needed — uses onlyunittest.mock+ minimal sys-modules stubs).manifest.yamlPATCH bump0.0.7 -> 0.0.8.tianditudoes not take user-suppliedhl/gl(the geocoder API takes akeyWordandregionfrom the request body, not as user params), so the third bug from PR #3456 (invoke fall-through on invalidhl/gl) does not apply.Change Type
LLM Plugin Checklist
Not applicable — this is a tool plugin.
Version
manifest.yamlis0.0.8.requests>=2.34.2is already declared and locked.Testing
python3 -m pytest tests/test_tianditu.py -v-> 11 passed.uv run --frozen ruff check .-> 1 pre-existing F401 (Unionunused intools/geocoder.py:2) is unchanged fromupstream/mainand unrelated to this PR; all new code added in this PR is clean.python3 -m py_compile tools/geocoder.py tools/staticmap.py tools/poisearch.py tests/test_tianditu.py-> OK.Risks
Out of scope (deliberate)
tools/tianditu/tools/staticmap.pydeclares a class namedPoiSearchTooleven though the file is actually a static-map image tool (not POI search). This is a pre-existing naming mistake onmainand is outside the scope of this bugfix PR.Unionunused import intools/geocoder.py:2is left alone to keep the diff strictly to the bug fixes.References
tools/google.tools/searchapiPR from this same rollout.tools/serperPR from this same rollout.tools/{searchapi, serper, tianditu}.