fix(tools): close S3 response bodies (#7647)

* fix(tools): close S3 response bodies

* fix(tools): preserve S3 reader outcomes

---------

Co-authored-by: gaoanze <gaoanze@meituan.com>
This commit is contained in:
gaoanze888
2026-09-21 18:41:45 +08:00
committed by GitHub
parent 0374c63129
commit 9c1275cc8f
2 changed files with 115 additions and 2 deletions

View File

@@ -1,3 +1,4 @@
from contextlib import suppress
import os
from crewai.tools import BaseTool
@@ -38,8 +39,13 @@ class S3ReaderTool(BaseTool):
)
response = s3.get_object(Bucket=bucket_name, Key=object_key)
result: str = response["Body"].read().decode("utf-8")
return result
body = response["Body"]
try:
result: str = body.read().decode("utf-8")
return result
finally:
with suppress(Exception):
body.close()
except ClientError as e:
return f"Error reading file from S3: {e!s}"

View File

@@ -0,0 +1,107 @@
"""Tests for the S3 reader tool."""
import sys
from types import ModuleType
from unittest.mock import Mock, patch
import pytest
from crewai_tools.aws.s3.reader_tool import S3ReaderTool
def _boto_modules(client: Mock) -> dict[str, ModuleType]:
"""Build minimal boto modules for exercising the lazy imports."""
boto3 = ModuleType("boto3")
boto3.client = Mock(return_value=client) # type: ignore[attr-defined]
botocore = ModuleType("botocore")
exceptions = ModuleType("botocore.exceptions")
class ClientError(Exception):
pass
exceptions.ClientError = ClientError # type: ignore[attr-defined]
botocore.exceptions = exceptions # type: ignore[attr-defined]
return {
"boto3": boto3,
"botocore": botocore,
"botocore.exceptions": exceptions,
}
def test_s3_reader_closes_response_body() -> None:
"""Release the streaming response after a successful read."""
body = Mock()
body.read.return_value = b"hello"
client = Mock()
client.get_object.return_value = {"Body": body}
with patch.dict(sys.modules, _boto_modules(client)):
result = S3ReaderTool()._run("s3://bucket/key.txt")
assert result == "hello"
body.close.assert_called_once_with()
def test_s3_reader_closes_response_body_after_decode_error() -> None:
"""Release the streaming response when UTF-8 decoding fails."""
body = Mock()
body.read.return_value = b"\xff"
client = Mock()
client.get_object.return_value = {"Body": body}
with (
patch.dict(sys.modules, _boto_modules(client)),
pytest.raises(UnicodeDecodeError),
):
S3ReaderTool()._run("s3://bucket/key.txt")
body.close.assert_called_once_with()
def test_s3_reader_closes_response_body_after_read_error() -> None:
"""Release the streaming response when reading the body fails."""
body = Mock()
body.read.side_effect = OSError("connection reset")
client = Mock()
client.get_object.return_value = {"Body": body}
with (
patch.dict(sys.modules, _boto_modules(client)),
pytest.raises(OSError, match="connection reset"),
):
S3ReaderTool()._run("s3://bucket/key.txt")
body.close.assert_called_once_with()
def test_s3_reader_preserves_result_when_close_fails() -> None:
"""Keep a successful read result when best-effort cleanup fails."""
body = Mock()
body.read.return_value = b"hello"
body.close.side_effect = OSError("close failed")
client = Mock()
client.get_object.return_value = {"Body": body}
with patch.dict(sys.modules, _boto_modules(client)):
result = S3ReaderTool()._run("s3://bucket/key.txt")
assert result == "hello"
body.close.assert_called_once_with()
def test_s3_reader_preserves_read_error_when_close_fails() -> None:
"""Keep the primary read error when best-effort cleanup also fails."""
body = Mock()
body.read.side_effect = OSError("connection reset")
body.close.side_effect = RuntimeError("close failed")
client = Mock()
client.get_object.return_value = {"Body": body}
with (
patch.dict(sys.modules, _boto_modules(client)),
pytest.raises(OSError, match="connection reset"),
):
S3ReaderTool()._run("s3://bucket/key.txt")
body.close.assert_called_once_with()