feat(transport): Add a timeout (#4252)

For some reason, we don't define any timeouts in our default
transport(s).

With this change:
- We add a 30s total timeout for the whole connect+read cycle in the
default HTTP transport
- In the experimental HTTP/2 httpcore-based transport there is no way to
set a single timeout, so we set 15s each for getting a connection from
the pool, connecting, writing, and reading

Backend SDKs in general set wildly different timeouts, from 30s in Go to
<5s in Ruby or PHP. I went for the higher end of the range here since
this is mainly meant to prevent the SDK preventing process shutdown like
described in #4247 --
we don't want to cut off legitimate requests that are just taking a long
time. (I was considering going even higher, maybe to 60s -- but I think
30s is a good first shot at this and we can always change it later.)

Author: sentrivana

sentry_sdk/transport.py

@@ -196,6 +196,8 @@ def _parse_rate_limits(header, now=None):
 class BaseHttpTransport(Transport):
     """The base HTTP transport."""
 
+    TIMEOUT = 30  # seconds
+
     def __init__(self, options):
         # type: (Self, Dict[str, Any]) -> None
         from sentry_sdk.consts import VERSION
@@ -621,6 +623,7 @@ def _get_pool_options(self):
         options = {
             "num_pools": 2 if num_pools is None else int(num_pools),
             "cert_reqs": "CERT_REQUIRED",
+            "timeout": urllib3.Timeout(total=self.TIMEOUT),
         }
 
         socket_options = None  # type: Optional[List[Tuple[int, int, int | bytes]]]
@@ -736,6 +739,8 @@ def __init__(self, options):
     class Http2Transport(BaseHttpTransport):  # type: ignore
         """The HTTP2 transport based on httpcore."""
 
+        TIMEOUT = 15
+
         if TYPE_CHECKING:
             _pool: Union[
                 httpcore.SOCKSProxy, httpcore.HTTPProxy, httpcore.ConnectionPool
@@ -765,6 +770,14 @@ def _request(
                 self._auth.get_api_url(endpoint_type),
                 content=body,
                 headers=headers,  # type: ignore
+                extensions={
+                    "timeout": {
+                        "pool": self.TIMEOUT,
+                        "connect": self.TIMEOUT,
+                        "write": self.TIMEOUT,
+                        "read": self.TIMEOUT,
+                    }
+                },
             )
             return response
 

tests/test_transport.py

@@ -14,6 +14,11 @@
 from pytest_localserver.http import WSGIServer
 from werkzeug.wrappers import Request, Response
 
+try:
+    import httpcore
+except (ImportError, ModuleNotFoundError):
+    httpcore = None
+
 try:
     import gevent
 except ImportError:
@@ -274,6 +279,37 @@ def test_keep_alive_on_by_default(make_client):
     assert "socket_options" not in options
 
 
+def test_default_timeout(make_client):
+    client = make_client()
+
+    options = client.transport._get_pool_options()
+    assert "timeout" in options
+    assert options["timeout"].total == client.transport.TIMEOUT
+
+
+@pytest.mark.skipif(not PY38, reason="HTTP2 libraries are only available in py3.8+")
+def test_default_timeout_http2(make_client):
+    client = make_client(_experiments={"transport_http2": True})
+
+    with mock.patch(
+        "sentry_sdk.transport.httpcore.ConnectionPool.request",
+        return_value=httpcore.Response(200),
+    ) as request_mock:
+        sentry_sdk.get_global_scope().set_client(client)
+        capture_message("hi")
+        client.flush()
+
+    request_mock.assert_called_once()
+    assert request_mock.call_args.kwargs["extensions"] == {
+        "timeout": {
+            "pool": client.transport.TIMEOUT,
+            "connect": client.transport.TIMEOUT,
+            "write": client.transport.TIMEOUT,
+            "read": client.transport.TIMEOUT,
+        }
+    }
+
+
 @pytest.mark.skipif(not PY38, reason="HTTP2 libraries are only available in py3.8+")
 def test_http2_with_https_dsn(make_client):
     client = make_client(_experiments={"transport_http2": True})