diff --git a/sentry_sdk/transport.py b/sentry_sdk/transport.py index a43ecabfb6..1b1842d03e 100644 --- a/sentry_sdk/transport.py +++ b/sentry_sdk/transport.py @@ -215,15 +215,7 @@ def __init__(self, options): ) # type: DefaultDict[Tuple[EventDataCategory, str], int] self._last_client_report_sent = time.time() - self._pool = self._make_pool( - self.parsed_dsn, - http_proxy=options["http_proxy"], - https_proxy=options["https_proxy"], - ca_certs=options["ca_certs"], - cert_file=options["cert_file"], - key_file=options["key_file"], - proxy_headers=options["proxy_headers"], - ) + self._pool = self._make_pool() # Backwards compatibility for deprecated `self.hub_class` attribute self._hub_cls = sentry_sdk.Hub @@ -532,8 +524,8 @@ def _serialize_envelope(self, envelope): return content_encoding, body - def _get_pool_options(self, ca_certs, cert_file=None, key_file=None): - # type: (Self, Optional[Any], Optional[Any], Optional[Any]) -> Dict[str, Any] + def _get_pool_options(self): + # type: (Self) -> Dict[str, Any] raise NotImplementedError() def _in_no_proxy(self, parsed_dsn): @@ -547,17 +539,8 @@ def _in_no_proxy(self, parsed_dsn): return True return False - def _make_pool( - self, - parsed_dsn, # type: Dsn - http_proxy, # type: Optional[str] - https_proxy, # type: Optional[str] - ca_certs, # type: Optional[Any] - cert_file, # type: Optional[Any] - key_file, # type: Optional[Any] - proxy_headers, # type: Optional[Dict[str, str]] - ): - # type: (...) -> Union[PoolManager, ProxyManager, httpcore.SOCKSProxy, httpcore.HTTPProxy, httpcore.ConnectionPool] + def _make_pool(self): + # type: (Self) -> Union[PoolManager, ProxyManager, httpcore.SOCKSProxy, httpcore.HTTPProxy, httpcore.ConnectionPool] raise NotImplementedError() def _request( @@ -631,8 +614,8 @@ class HttpTransport(BaseHttpTransport): if TYPE_CHECKING: _pool: Union[PoolManager, ProxyManager] - def _get_pool_options(self, ca_certs, cert_file=None, key_file=None): - # type: (Self, Any, Any, Any) -> Dict[str, Any] + def _get_pool_options(self): + # type: (Self) -> Dict[str, Any] num_pools = self.options.get("_experiments", {}).get("transport_num_pools") options = { @@ -658,42 +641,43 @@ def _get_pool_options(self, ca_certs, cert_file=None, key_file=None): options["socket_options"] = socket_options options["ca_certs"] = ( - ca_certs # User-provided bundle from the SDK init + self.options["ca_certs"] # User-provided bundle from the SDK init or os.environ.get("SSL_CERT_FILE") or os.environ.get("REQUESTS_CA_BUNDLE") or certifi.where() ) - options["cert_file"] = cert_file or os.environ.get("CLIENT_CERT_FILE") - options["key_file"] = key_file or os.environ.get("CLIENT_KEY_FILE") + options["cert_file"] = self.options["cert_file"] or os.environ.get( + "CLIENT_CERT_FILE" + ) + options["key_file"] = self.options["key_file"] or os.environ.get( + "CLIENT_KEY_FILE" + ) return options - def _make_pool( - self, - parsed_dsn, # type: Dsn - http_proxy, # type: Optional[str] - https_proxy, # type: Optional[str] - ca_certs, # type: Any - cert_file, # type: Any - key_file, # type: Any - proxy_headers, # type: Optional[Dict[str, str]] - ): - # type: (...) -> Union[PoolManager, ProxyManager] + def _make_pool(self): + # type: (Self) -> Union[PoolManager, ProxyManager] + if self.parsed_dsn is None: + raise ValueError("Cannot create HTTP-based transport without valid DSN") + proxy = None - no_proxy = self._in_no_proxy(parsed_dsn) + no_proxy = self._in_no_proxy(self.parsed_dsn) # try HTTPS first - if parsed_dsn.scheme == "https" and (https_proxy != ""): + https_proxy = self.options["https_proxy"] + if self.parsed_dsn.scheme == "https" and (https_proxy != ""): proxy = https_proxy or (not no_proxy and getproxies().get("https")) # maybe fallback to HTTP proxy + http_proxy = self.options["http_proxy"] if not proxy and (http_proxy != ""): proxy = http_proxy or (not no_proxy and getproxies().get("http")) - opts = self._get_pool_options(ca_certs, cert_file, key_file) + opts = self._get_pool_options() if proxy: + proxy_headers = self.options["proxy_headers"] if proxy_headers: opts["proxy_headers"] = proxy_headers @@ -783,10 +767,11 @@ def _request( ) return response - def _get_pool_options(self, ca_certs, cert_file=None, key_file=None): - # type: (Any, Any, Any) -> Dict[str, Any] + def _get_pool_options(self): + # type: (Self) -> Dict[str, Any] options = { - "http2": True, + "http2": self.parsed_dsn is not None + and self.parsed_dsn.scheme == "https", "retries": 3, } # type: Dict[str, Any] @@ -805,13 +790,13 @@ def _get_pool_options(self, ca_certs, cert_file=None, key_file=None): ssl_context = ssl.create_default_context() ssl_context.load_verify_locations( - ca_certs # User-provided bundle from the SDK init + self.options["ca_certs"] # User-provided bundle from the SDK init or os.environ.get("SSL_CERT_FILE") or os.environ.get("REQUESTS_CA_BUNDLE") or certifi.where() ) - cert_file = cert_file or os.environ.get("CLIENT_CERT_FILE") - key_file = key_file or os.environ.get("CLIENT_KEY_FILE") + cert_file = self.options["cert_file"] or os.environ.get("CLIENT_CERT_FILE") + key_file = self.options["key_file"] or os.environ.get("CLIENT_KEY_FILE") if cert_file is not None: ssl_context.load_cert_chain(cert_file, key_file) @@ -819,31 +804,27 @@ def _get_pool_options(self, ca_certs, cert_file=None, key_file=None): return options - def _make_pool( - self, - parsed_dsn, # type: Dsn - http_proxy, # type: Optional[str] - https_proxy, # type: Optional[str] - ca_certs, # type: Any - cert_file, # type: Any - key_file, # type: Any - proxy_headers, # type: Optional[Dict[str, str]] - ): - # type: (...) -> Union[httpcore.SOCKSProxy, httpcore.HTTPProxy, httpcore.ConnectionPool] + def _make_pool(self): + # type: (Self) -> Union[httpcore.SOCKSProxy, httpcore.HTTPProxy, httpcore.ConnectionPool] + if self.parsed_dsn is None: + raise ValueError("Cannot create HTTP-based transport without valid DSN") proxy = None - no_proxy = self._in_no_proxy(parsed_dsn) + no_proxy = self._in_no_proxy(self.parsed_dsn) # try HTTPS first - if parsed_dsn.scheme == "https" and (https_proxy != ""): + https_proxy = self.options["https_proxy"] + if self.parsed_dsn.scheme == "https" and (https_proxy != ""): proxy = https_proxy or (not no_proxy and getproxies().get("https")) # maybe fallback to HTTP proxy + http_proxy = self.options["http_proxy"] if not proxy and (http_proxy != ""): proxy = http_proxy or (not no_proxy and getproxies().get("http")) - opts = self._get_pool_options(ca_certs, cert_file, key_file) + opts = self._get_pool_options() if proxy: + proxy_headers = self.options["proxy_headers"] if proxy_headers: opts["proxy_headers"] = proxy_headers diff --git a/tests/test_transport.py b/tests/test_transport.py index 2e4b36afd4..d24bea0491 100644 --- a/tests/test_transport.py +++ b/tests/test_transport.py @@ -219,7 +219,7 @@ def test_transport_num_pools(make_client, num_pools, expected_num_pools): client = make_client(_experiments=_experiments) - options = client.transport._get_pool_options([]) + options = client.transport._get_pool_options() assert options["num_pools"] == expected_num_pools @@ -231,12 +231,15 @@ def test_two_way_ssl_authentication(make_client, http2): if http2: _experiments["transport_http2"] = True - client = make_client(_experiments=_experiments) - current_dir = os.path.dirname(__file__) cert_file = f"{current_dir}/test.pem" key_file = f"{current_dir}/test.key" - options = client.transport._get_pool_options([], cert_file, key_file) + client = make_client( + cert_file=cert_file, + key_file=key_file, + _experiments=_experiments, + ) + options = client.transport._get_pool_options() if http2: assert options["ssl_context"] is not None @@ -254,23 +257,39 @@ def test_socket_options(make_client): client = make_client(socket_options=socket_options) - options = client.transport._get_pool_options([]) + options = client.transport._get_pool_options() assert options["socket_options"] == socket_options def test_keep_alive_true(make_client): client = make_client(keep_alive=True) - options = client.transport._get_pool_options([]) + options = client.transport._get_pool_options() assert options["socket_options"] == KEEP_ALIVE_SOCKET_OPTIONS def test_keep_alive_on_by_default(make_client): client = make_client() - options = client.transport._get_pool_options([]) + options = client.transport._get_pool_options() assert "socket_options" not in options +@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}) + client.transport.parsed_dsn.scheme = "https" + options = client.transport._get_pool_options() + assert options["http2"] is True + + +@pytest.mark.skipif(not PY38, reason="HTTP2 libraries are only available in py3.8+") +def test_no_http2_with_http_dsn(make_client): + client = make_client(_experiments={"transport_http2": True}) + client.transport.parsed_dsn.scheme = "http" + options = client.transport._get_pool_options() + assert options["http2"] is False + + def test_socket_options_override_keep_alive(make_client): socket_options = [ (socket.SOL_SOCKET, socket.SO_KEEPALIVE, 1), @@ -280,7 +299,7 @@ def test_socket_options_override_keep_alive(make_client): client = make_client(socket_options=socket_options, keep_alive=False) - options = client.transport._get_pool_options([]) + options = client.transport._get_pool_options() assert options["socket_options"] == socket_options @@ -292,7 +311,7 @@ def test_socket_options_merge_with_keep_alive(make_client): client = make_client(socket_options=socket_options, keep_alive=True) - options = client.transport._get_pool_options([]) + options = client.transport._get_pool_options() try: assert options["socket_options"] == [ (socket.SOL_SOCKET, socket.SO_KEEPALIVE, 42), @@ -314,7 +333,7 @@ def test_socket_options_override_defaults(make_client): # socket option defaults, so we need to set this and not ignore it. client = make_client(socket_options=[]) - options = client.transport._get_pool_options([]) + options = client.transport._get_pool_options() assert options["socket_options"] == []