mirror of
https://github.com/usestrix/strix.git
synced 2026-10-05 02:41:38 +00:00
fix(telemetry): match only http.client responses in the finalizer filter; test PTY teardown through the real SDK session
The object-shape match accepted any http.* class; it now takes urllib3 and http.client only, so a closed-file error from another module still reaches the previous unraisable hook. New tests build a real DockerSandboxSession holding a PTY exec stream and run the real SDK delete() with the container gone: the SDK alone leaves the socket and its pinned response open, StrixDockerSandboxClient.delete() closes both.
This commit is contained in:
parent
b4be726a3b
commit
d945eeee32
3 changed files with 82 additions and 8 deletions
|
|
@ -95,7 +95,6 @@ def configure_dependency_logging() -> None:
|
|||
_unraisable_hook_installed = False
|
||||
|
||||
|
||||
_FINALIZER_NOISE_MODULES = frozenset({"urllib3", "http"})
|
||||
_FINALIZER_FILE_RE = re.compile(r"finalizing file <(urllib3|http\.client)\.")
|
||||
|
||||
|
||||
|
|
@ -112,7 +111,8 @@ def _is_urllib3_closed_file_noise(unraisable: sys.UnraisableHookArgs) -> bool:
|
|||
):
|
||||
return False
|
||||
if unraisable.object is not None:
|
||||
return type(unraisable.object).__module__.split(".")[0] in _FINALIZER_NOISE_MODULES
|
||||
module = type(unraisable.object).__module__
|
||||
return module.split(".")[0] == "urllib3" or module == "http.client"
|
||||
return _FINALIZER_FILE_RE.search(unraisable.err_msg or "") is not None
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -11,22 +11,28 @@ would let it escape and surface a traceback on every teardown.
|
|||
|
||||
from __future__ import annotations
|
||||
|
||||
import uuid
|
||||
from pathlib import Path
|
||||
from types import SimpleNamespace
|
||||
from typing import TYPE_CHECKING, cast
|
||||
from typing import cast
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
from agents.sandbox.sandboxes.docker import DockerSandboxClient
|
||||
from agents.sandbox.manifest import Manifest
|
||||
from agents.sandbox.sandboxes.docker import (
|
||||
DockerSandboxClient,
|
||||
DockerSandboxSession,
|
||||
DockerSandboxSessionState,
|
||||
_DockerExecSocket,
|
||||
_DockerPtyProcessEntry,
|
||||
)
|
||||
from agents.sandbox.session.sandbox_session import SandboxSession
|
||||
from docker import errors as docker_errors
|
||||
from requests.exceptions import ConnectionError as RequestsConnectionError
|
||||
|
||||
from strix.runtime.docker_client import StrixDockerSandboxClient
|
||||
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from agents.sandbox.session.sandbox_session import SandboxSession
|
||||
|
||||
|
||||
def _client_with_kill_error(exc: Exception) -> StrixDockerSandboxClient:
|
||||
"""A StrixDockerSandboxClient whose containers.get(...).kill() raises ``exc``."""
|
||||
client = StrixDockerSandboxClient.__new__(StrixDockerSandboxClient)
|
||||
|
|
@ -134,3 +140,65 @@ async def test_delete_survives_pty_termination_errors() -> None:
|
|||
await client.delete(session)
|
||||
|
||||
super_delete.assert_awaited_once()
|
||||
|
||||
|
||||
def _real_session_with_open_pty(
|
||||
container_id: str = "abc123",
|
||||
) -> tuple[SandboxSession, _DockerExecSocket]:
|
||||
"""A real SDK DockerSandboxSession holding one live PTY exec stream, the
|
||||
shape exec_command leaves behind: a hijacked socket plus the streamed HTTP
|
||||
response docker-py pins to it."""
|
||||
state = DockerSandboxSessionState.model_construct(
|
||||
type="docker",
|
||||
image="sandbox:test",
|
||||
container_id=container_id,
|
||||
manifest=Manifest(),
|
||||
session_id=uuid.uuid4(),
|
||||
exposed_ports=(),
|
||||
workspace_root_ready=True,
|
||||
)
|
||||
container = MagicMock()
|
||||
container.client.api.exec_inspect.return_value = {"Running": False, "ExitCode": 0}
|
||||
inner = DockerSandboxSession(docker_client=MagicMock(), container=container, state=state)
|
||||
exec_socket = _DockerExecSocket(sock=MagicMock(), raw_sock=MagicMock(), response=MagicMock())
|
||||
inner._pty_processes[1] = _DockerPtyProcessEntry(
|
||||
exec_id="exec-1",
|
||||
sock=exec_socket,
|
||||
raw_sock=exec_socket.raw_sock,
|
||||
pid_path=Path("/workspace/.pty/1.pid"),
|
||||
tty=True,
|
||||
)
|
||||
return SandboxSession(inner), exec_socket
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_sdk_delete_alone_leaves_pty_exec_streams_open_when_the_container_is_gone() -> None:
|
||||
"""Documents the SDK gap delete() compensates for: with the container gone,
|
||||
DockerSandboxClient.delete() never terminates the PTY entries."""
|
||||
client = StrixDockerSandboxClient.__new__(StrixDockerSandboxClient)
|
||||
client.docker_client = MagicMock()
|
||||
client.docker_client.containers.get.side_effect = docker_errors.NotFound("gone")
|
||||
session, exec_socket = _real_session_with_open_pty()
|
||||
|
||||
await DockerSandboxClient.delete(client, session)
|
||||
|
||||
assert session._inner._pty_processes # type: ignore[attr-defined]
|
||||
cast("MagicMock", exec_socket.sock).close.assert_not_called()
|
||||
cast("MagicMock", exec_socket.response).close.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_delete_closes_real_pty_exec_streams_when_the_container_is_gone() -> None:
|
||||
"""End to end through the real SDK session and the real SDK delete(): the
|
||||
exec socket and its pinned HTTP response are closed, so nothing is left for
|
||||
the garbage collector at interpreter exit."""
|
||||
client = StrixDockerSandboxClient.__new__(StrixDockerSandboxClient)
|
||||
client.docker_client = MagicMock()
|
||||
client.docker_client.containers.get.side_effect = docker_errors.NotFound("gone")
|
||||
session, exec_socket = _real_session_with_open_pty()
|
||||
|
||||
await client.delete(session)
|
||||
|
||||
assert not session._inner._pty_processes # type: ignore[attr-defined]
|
||||
cast("MagicMock", exec_socket.sock).close.assert_called_once()
|
||||
cast("MagicMock", exec_socket.response).close.assert_called_once()
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import http.client
|
||||
import http.cookiejar
|
||||
import socket
|
||||
import sys
|
||||
|
||||
|
|
@ -29,6 +30,11 @@ def test_filters_urllib3_closed_file_noise() -> None:
|
|||
assert _is_urllib3_closed_file_noise(args) # type: ignore[arg-type]
|
||||
|
||||
|
||||
def test_ignores_closed_file_errors_from_other_http_modules() -> None:
|
||||
args = _Args(ValueError("I/O operation on closed file."), http.cookiejar.CookieJar())
|
||||
assert not _is_urllib3_closed_file_noise(args) # type: ignore[arg-type]
|
||||
|
||||
|
||||
def test_filters_http_client_closed_file_noise() -> None:
|
||||
sock = socket.socket()
|
||||
try:
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue