Repository navigation
Commit ec4cb80
fix(tools): bind JSONDecodeError in ToolConnectionAnalyzer.analyze
Merge google#5942
**Please ensure you have read the [contribution guide](https://github.com/google/adk-python/blob/main/CONTRIBUTING.md) before creating a pull request.**
### Link to Issue or Description of Change
**2. Or, if no issue exists, describe the change:**
**Problem:**
`ToolConnectionAnalyzer.analyze()` in
`src/google/adk/tools/environment_simulation/tool_connection_analyzer.py`
crashes whenever the analyzed LLM returns a response that is not valid JSON.
The JSON parse is wrapped in a `try/except`, but the `except` clause does not
bind the exception:
```python
except json.JSONDecodeError:
logging.warning(
"Failed to parse tool connection analysis from LLM. Proceeding"
" without connection map. Error: %s\nLLM Output:\n%s",
e, # <-- 'e' was never defined
response_text,
)
return ToolConnectionMap(stateful_parameters=[])
```
The warning log references `e`, but the `except` clause binds nothing. So the
moment a non-JSON response is parsed, the handler itself raises
`NameError: name 'e' is not defined`. This masks the real parse error and
crashes `analyze()` instead of degrading gracefully to an empty
`ToolConnectionMap` as the surrounding code clearly intends.
Reproduced traceback (LLM mocked to return `"this is not json at all"`):
```text
File ".../tool_connection_analyzer.py", line 136, in analyze
response_json = json.loads(clean_json_text.strip())
...
json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)
During handling of the above exception, another exception occurred:
File ".../tool_connection_analyzer.py", line 141, in analyze
e,
^
NameError: name 'e' is not defined
```
**Solution:**
Bind the caught exception with `as e` so the handler can log the real parse
error and return an empty `ToolConnectionMap` as designed:
```python
except json.JSONDecodeError as e:
logging.warning(...)
return ToolConnectionMap(stateful_parameters=[])
```
This is a one-character fix; the surrounding logging and fallback behaviour are
unchanged. After the fix, the same input logs the underlying parse error and
returns `ToolConnectionMap(stateful_parameters=[])` without raising.
### Testing Plan
**Unit Tests:**
- [x] I have added or updated unit tests for my change.
- [x] All unit tests pass locally.
This branch of `analyze()` previously had zero coverage, so I added
`tests/unittests/tools/environment_simulation/test_tool_connection_analyzer.py`
covering:
- the malformed-JSON path (regression guard — fails with `NameError` on the
pre-fix code, passes after the fix),
- the valid-JSON path, and
- the Markdown code-fence stripping path.
`pytest` summary (low parallelism, as run locally):
```text
$ pytest tests/unittests/tools/environment_simulation/ -n2
======================= 12 passed, 11 warnings in 1.64s ========================
```
```text
$ pytest tests/unittests/tools/environment_simulation/test_tool_connection_analyzer.py -n0 -v
...test_malformed_json_returns_empty_map_without_crashing PASSED
...test_valid_json_is_parsed_into_connection_map PASSED
...test_fenced_json_is_stripped_before_parsing PASSED
======================== 3 passed, 5 warnings in 0.57s =========================
```
Verified the regression test fails on the unfixed code with the exact
`NameError: name 'e' is not defined` and passes after the fix. `isort` and
`pyink` report no changes on the modified files.
**Manual End-to-End (E2E) Tests:**
Reproduced directly against the analyzer with a mocked LLM:
```python
analyzer = ToolConnectionAnalyzer(llm_name=..., llm_config=...)
# LLM mocked to return a non-JSON string.
result = await analyzer.analyze([some_tool])
```
- Before the fix: raises `NameError: name 'e' is not defined`.
- After the fix: logs
`Failed to parse tool connection analysis from LLM... Error: Expecting value: line 1 column 1 (char 0)`
and returns `ToolConnectionMap(stateful_parameters=[])`.
### Checklist
- [x] I have read the [CONTRIBUTING.md](https://github.com/google/adk-python/blob/main/CONTRIBUTING.md) document.
- [x] I have performed a self-review of my own code.
- [x] I have commented my code, particularly in hard-to-understand areas.
- [x] I have added tests that prove my fix is effective or that my feature works.
- [x] New and existing unit tests pass locally with my changes.
- [x] I have manually tested my changes end-to-end.
- [ ] Any dependent changes have been merged and published in downstream modules.
### Additional context
The affected module is marked `@experimental(FeatureName.ENVIRONMENT_SIMULATION)`.
Co-authored-by: George Weale <gweale@google.com>
COPYBARA_INTEGRATE_REVIEW=google#5942 from sarathfrancis90:fix-tool-connection-analyzer-jsondecodeerror 536dd56
PiperOrigin-RevId: 9553736981 parent f070262 commit ec4cb80
2 files changed
Lines changed: 120 additions & 1 deletion
File tree
- src/google/adk/tools/environment_simulation
- tests/unittests/tools/environment_simulation
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
130 | 130 | | |
131 | 131 | | |
132 | 132 | | |
133 | | - | |
| 133 | + | |
134 | 134 | | |
135 | 135 | | |
136 | 136 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
0 commit comments