Skip to content

fix(tools): handle non-UTF-8 HTTP error responses - #7023

Open
SUJALMU2004 wants to merge 1 commit into
google:mainfrom
SUJALMU2004:fix/rest-api-error-decoding
Open

fix(tools): handle non-UTF-8 HTTP error responses#7023
SUJALMU2004 wants to merge 1 commit into
google:mainfrom
SUJALMU2004:fix/rest-api-error-decoding

Conversation

@SUJALMU2004

@SUJALMU2004 SUJALMU2004 commented Sep 5, 2026

Copy link
Copy Markdown

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

No matching existing issue was found.

2. Or, if no issue exists, describe the change:

The bug report, reproduction, and fix are provided below, as permitted by CONTRIBUTING.md.

Problem: RestApiTool decodes HTTP error bodies with response.content.decode("utf-8"). An API that returns a Latin-1 error with a valid Content-Type charset, or an error containing invalid UTF-8 bytes, raises UnicodeDecodeError instead of returning the tool's normal error dictionary. The decoding error masks the HTTP status and message.

Solution: Use response.text, matching the existing non-JSON success path. HTTPX honors the response charset and replaces undecodable bytes. No new dependency or public API change.

Environment: Reproduced on upstream 25f5214c83f56b2fcffd35757e886026632f3c2b (ADK 2.8.0), Windows 11, Python 3.12.10, HTTPX 0.28.1. No model or LiteLLM is required.

Steps to reproduce: Run the local HTTP script below against upstream. The first endpoint responds with HTTP 400, Content-Type: text/plain; charset=iso-8859-1, and b"Acc\xe8s refus\xe9".

  • Expected: an error dictionary containing HTTP 400 and the decoded message.
  • Observed before the fix: UnicodeDecodeError: 'utf-8' codec can't decode byte 0xe8 in position 3: invalid continuation byte.
  • After the fix: the error dictionary is returned, and all four scenarios pass.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

Added three regression cases through RestApiTool.run_async, with a real HTTPX client and MockTransport only at the network boundary. They cover declared Latin-1, invalid UTF-8, and a missing charset. The existing ASCII HTTP error test retains its exact result assertion and now uses a real httpx.Response.

The affected tests pass on every supported Python version. The full-repository checkbox remains unchecked because the complete tox matrix is not verified; see the limitation below.

Regression proof on unmodified source: 3 failed, 1 passed; all three new cases fail with UnicodeDecodeError.

With the fix:

python -m pytest tests/unittests/tools/openapi_tool -q --tb=short
282 passed, 27 warnings in 32.44s

The built wheel also passes all 66 tests in test_rest_api_tool.py in isolated environments on Python 3.10, 3.11, 3.13, and 3.14 (Python 3.12 is covered by the full OpenAPI suite above):

uv run --isolated --no-project --python <version> --with ./dist/google_adk-2.8.0-py3-none-any.whl --with pytest --with pytest-asyncio --with pytest-mock python -m pytest tests/unittests/tools/openapi_tool/openapi_spec_parser/test_rest_api_tool.py -q --tb=short
Python 3.10: 66 passed, 22 warnings
Python 3.11: 66 passed, 22 warnings
Python 3.13: 66 passed, 21 warnings
Python 3.14: 66 passed, 22 warnings

Full-suite limitation: Attempted the repository's full tox matrix on Windows, then interrupted the Python 3.10 run after failures in untouched CLI tests. A separate rerun of tests/unittests/cli/conformance/test_generate_markdown_utils.py -x confirmed UnicodeEncodeError when _generate_markdown_utils.py:112 writes a checkmark using the Windows cp1252 encoding. Both that source file and its tests are unchanged from the base commit. The complete full-suite matrix is not verified. GitHub CI currently requires maintainer approval for this first contribution.

Formatting and checks: Ruff, isort, pyink, whitespace, license, compliance, and codespell hooks passed. On Windows the check-new-py-prefix wrapper fails to launch because its shebang uses /bin/bash. Its exact underlying check passed with python scripts/check_new_py_files.py --new-dir .; only that wrapper was skipped for the commit.

Manual End-to-End (E2E) Tests:

uv build succeeded. Ran the script below both from the editable checkout and against the built wheel in a clean environment:

uv run --isolated --no-project --python 3.12 --with ./dist/google_adk-2.8.0-py3-none-any.whl python ../verify_rest_api_error_decoding.py
PASS /latin1 HTTP 400
PASS /invalid HTTP 502
PASS /ascii HTTP 500
PASS /success HTTP 200

Each scenario asserts the returned status/message or successful JSON payload. The script uses a real loopback HTTP server, with no cloud credentials or model calls.

Reproduction and local HTTP verification script

Save as ../verify_rest_api_error_decoding.py:

"""Exercise RestApiTool against a real local HTTP server, without an LLM."""

import asyncio
from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
import json
from threading import Thread

from fastapi.openapi.models import Operation
from google.adk.tools.openapi_tool.openapi_spec_parser.openapi_spec_parser import OperationEndpoint
from google.adk.tools.openapi_tool.openapi_spec_parser.rest_api_tool import RestApiTool


CASES = {
    "/latin1": (400, "text/plain; charset=iso-8859-1", b"Acc\xe8s refus\xe9", "Acc\u00e8s refus\u00e9"),
    "/invalid": (502, "text/plain; charset=utf-8", b"Invalid byte: \xff", "Invalid byte: \ufffd"),
    "/ascii": (500, "text/plain", b"Internal Server Error", "Internal Server Error"),
    "/success": (200, "application/json", b'{"ok": true}', None),
}


class Handler(BaseHTTPRequestHandler):
    def do_GET(self):
        status, content_type, body, _ = CASES[self.path]
        self.send_response(status)
        self.send_header("Content-Type", content_type)
        self.send_header("Content-Length", str(len(body)))
        self.end_headers()
        self.wfile.write(body)

    def log_message(self, *_):
        pass


async def verify(port):
    for path, (status, _, _, expected_text) in CASES.items():
        tool = RestApiTool(
            name="test_tool",
            description="Local HTTP error decoding verification",
            endpoint=OperationEndpoint(
                base_url=f"http://127.0.0.1:{port}", path=path, method="GET"
            ),
            operation=Operation(operationId="testOperation"),
        )
        result = await tool.run_async(args={}, tool_context=None)
        if status >= 400:
            assert f"Status Code: {status}" in result["error"]
            assert expected_text in result["error"]
        else:
            assert result == {"ok": True}
        print(f"PASS {path} HTTP {status}: {json.dumps(result, ensure_ascii=True)}")


if __name__ == "__main__":
    with ThreadingHTTPServer(("127.0.0.1", 0), Handler) as server:
        thread = Thread(target=server.serve_forever, daemon=True)
        thread.start()
        try:
            asyncio.run(verify(server.server_port))
        finally:
            server.shutdown()
            thread.join()

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas. The new test includes a docstring explaining the expected behavior.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes. The affected suites pass; the complete repository matrix remains unverified as explained above.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules. Not applicable: there are no dependent changes.

No public API or documentation changes are needed.

Additional context

Prepared with Codex assistance. The Google CLA check has passed and this PR is ready for review. CI still requires maintainer approval; the full-suite validation limitation is documented above.

@SUJALMU2004
SUJALMU2004 marked this pull request as ready for review September 5, 2026 08:35
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.

2 participants