From bbe19d5d21cac8b5716008cceb3ea48f3f441df6 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Tue, 27 Oct 2026 23:46:40 +0100 Subject: [PATCH 1/2] feat(git): Report a refused git host as git_remote_refused Sentry builds its relay with BlacklistAdapter, which refuses a GitHub Enterprise or self-managed GitLab host that resolves to an internal address by raising its own RestrictedIPAddress from inside urllib3's connect. That error is neither coded nor a requests exception, so GitServer counted it as unhandled_exception and re-raised it, and Sentry had to catch, report and answer it itself. scm can't import Sentry's exception, so the caller names it in GitRelay(refused_host_errors=...), and the relay raises GitRemoteRefused in its place. GitServer then counts and answers it like its other refusals. The original is dropped with "from None" because its traceback runs through urllib3 frames that hold the Authorization header, and the new error says nothing of the address. The constructor checks the tuple because an except clause with anything else fails only when a request does, as a TypeError chained to that request's error. --- src/scm/errors.py | 6 ++++ src/scm/git/relay.py | 23 ++++++++++++--- src/scm/git/server.py | 6 ++-- tests/unit/git_upstream.py | 24 ++++++++++++++++ tests/unit/test_git_relay.py | 54 +++++++++++++++++++++++++++++------ tests/unit/test_git_server.py | 21 ++++++++++---- 6 files changed, 114 insertions(+), 20 deletions(-) diff --git a/src/scm/errors.py b/src/scm/errors.py index cb40781..59a9e33 100644 --- a/src/scm/errors.py +++ b/src/scm/errors.py @@ -6,6 +6,7 @@ "git_operation_not_granted", "git_provider_not_supported", "git_remote_not_https", + "git_remote_refused", "git_repository_not_granted", "git_route_not_supported", "git_token_rejected", @@ -53,6 +54,7 @@ "git_operation_not_granted": "The git grant does not allow this operation.", "git_provider_not_supported": "The repository's provider has no git remote.", "git_remote_not_https": "The repository's git remote is not an https URL.", + "git_remote_refused": "The repository's git remote is at an address Sentry does not connect to.", "git_repository_not_granted": "The git grant does not cover this repository.", "git_route_not_supported": "The request is not a git fetch or push.", "git_token_rejected": "The provider rejected Sentry's credentials for the repository.", @@ -169,6 +171,10 @@ class GitRemoteNotHttps(GitSCMCodedError): code = "git_remote_not_https" +class GitRemoteRefused(GitSCMCodedError): + code = "git_remote_refused" + + class GitRepositoryNotGranted(GitSCMCodedError): code = "git_repository_not_granted" diff --git a/src/scm/git/relay.py b/src/scm/git/relay.py index 1d60ed8..bd07aca 100644 --- a/src/scm/git/relay.py +++ b/src/scm/git/relay.py @@ -11,6 +11,7 @@ ClientClosedRequest, GitOperationNotGranted, GitRemoteNotHttps, + GitRemoteRefused, GitRepositoryNotGranted, GitRouteNotSupported, ResourceBadGateway, @@ -106,10 +107,12 @@ class GitRelay: ``adapter`` builds the adapter that sends each upstream request. Pass one that refuses internal addresses: a GitHub Enterprise or self-managed GitLab host is whatever the customer configured, and the relay connects wherever it resolves. Sentry's ``BlacklistAdapter`` does this for its API clients, and the class itself can be - passed. An error the adapter raises for a refused host reaches the caller unchanged, unless it is a requests or - urllib3 exception, which the relay replaces like any other so that it cannot carry the token. The adapter must - not retry, because a retry cannot resend a body that has already been streamed. Without one, the relay uses a plain - ``HTTPAdapter``, which checks nothing. + passed. ``refused_host_errors`` names the exception classes the adapter raises for a refused host, such as + Sentry's ``RestrictedIPAddress``; the relay raises ``GitRemoteRefused`` in their place, which GitServer answers + like its other refusals. Any other error the adapter raises reaches the caller unchanged, unless it is a requests + or urllib3 exception, which the relay replaces like any other so that it cannot carry the token. The adapter + must not retry, because a retry cannot resend a body that has already been streamed. Without one, the relay uses + a plain ``HTTPAdapter``, which checks nothing. The relay takes no settings from the environment. Through a proxy from ``HTTPS_PROXY``, the adapter would check the proxy's address rather than the provider's. ``ca_bundle`` names a CA file to trust instead of the @@ -120,11 +123,19 @@ def __init__( self, *, adapter: Callable[[], HTTPAdapter] = HTTPAdapter, + refused_host_errors: tuple[type[Exception], ...] = (), ca_bundle: str | None = None, timeout: tuple[float, float] = DEFAULT_TIMEOUT, chunk_size: int = DEFAULT_CHUNK_SIZE, ) -> None: + # Checked now rather than when a request fails: in an except clause, anything but a tuple of exception + # classes turns every failed request into a TypeError chained to the original error. + if not isinstance(refused_host_errors, tuple) or not all( + isinstance(error, type) and issubclass(error, Exception) for error in refused_host_errors + ): + raise TypeError("refused_host_errors must be a tuple of exception classes.") self.adapter = adapter + self.refused_host_errors = refused_host_errors self.ca_bundle = ca_bundle self.timeout = timeout self.chunk_size = chunk_size @@ -202,6 +213,10 @@ def forward( allow_redirects=False, timeout=self.timeout, ) + # First, in case the adapter refuses with a requests exception. "from None" drops the refusal: raised as the + # adapter connects, its traceback runs through urllib3's frames, which hold the Authorization header. + except self.refused_host_errors: + raise GitRemoteRefused() from None # "from None" drops requests' exception, whose request holds the Authorization header. except requests.Timeout: raise ResourceGatewayTimeout() from None diff --git a/src/scm/git/server.py b/src/scm/git/server.py index c8bdd0c..f2c5ced 100644 --- a/src/scm/git/server.py +++ b/src/scm/git/server.py @@ -33,6 +33,7 @@ "missing_repository_path": 500, # The provider cannot be reached safely, or failed. "git_remote_not_https": 502, + "git_remote_refused": 502, "git_token_rejected": 502, "git_token_unavailable": 502, "resource_bad_gateway": 502, @@ -59,8 +60,9 @@ class GitServer: when it failed, as tags: ``repository_id`` as requested, then ``organization_id`` and ``referrer`` once the grant is verified, ``operation``, ``provider`` and ``remote_host``. A tag is missing when the request failed before it was known. A client closing its request is not reported, since there is nothing to fix. Any other - exception, such as an adapter refusing a host or the token supplier failing, reaches the caller unchanged, so - the caller can answer it the way it answers the same error elsewhere. + exception, such as the token supplier failing, or the relay's adapter refusing a host with an error its + ``refused_host_errors`` does not name, reaches the caller unchanged, so the caller can answer it the way it + answers the same error elsewhere. Every count is tagged with the grant's ``referrer`` once the grant is verified, so Sentry can tell a coding job's clone from Seer's own fetch or push. diff --git a/tests/unit/git_upstream.py b/tests/unit/git_upstream.py index 71cdb30..38addcb 100644 --- a/tests/unit/git_upstream.py +++ b/tests/unit/git_upstream.py @@ -8,6 +8,7 @@ import datetime import functools import ipaddress +import socket import ssl import sys import tempfile @@ -23,6 +24,8 @@ from cryptography.x509.oid import NameOID from requests.adapters import HTTPAdapter from requests.structures import CaseInsensitiveDict +from urllib3 import HTTPSConnectionPool +from urllib3.connection import HTTPSConnection @dataclass @@ -138,6 +141,27 @@ def close(self) -> None: super().close() +class HostRefused(Exception): + """Stands in for Sentry's ``RestrictedIPAddress``, which is neither a requests nor a urllib3 exception.""" + + +class _RefusingConnection(HTTPSConnection): + def _new_conn(self) -> socket.socket: + raise HostRefused(f"{self.host} is an internal address") + + +class _RefusingPool(HTTPSConnectionPool): + ConnectionCls = _RefusingConnection + + +class RefusingAdapter(HTTPAdapter): + """Refuses every host as it connects, from inside urllib3 the way Sentry's ``BlacklistAdapter`` does.""" + + def init_poolmanager(self, *args, **kwargs) -> None: + super().init_poolmanager(*args, **kwargs) + self.poolmanager.pool_classes_by_scheme = {"https": _RefusingPool} + + def echo(handler: UpstreamHandler) -> None: body = b"".join(handler.body_chunks()) handler.record(body) diff --git a/tests/unit/test_git_relay.py b/tests/unit/test_git_relay.py index 6f54b98..d7fe48c 100644 --- a/tests/unit/test_git_relay.py +++ b/tests/unit/test_git_relay.py @@ -14,6 +14,7 @@ from scm.errors import ( GitOperationNotGranted, GitRemoteNotHttps, + GitRemoteRefused, GitRepositoryNotGranted, GitRouteNotSupported, ResourceBadGateway, @@ -25,7 +26,15 @@ from scm.git.relay import GitRelay, _UpstreamStream, authorize_request from scm.types import GitRemote from tests.unit.git_builders import make_grant -from tests.unit.git_upstream import ClosingAdapter, UpstreamHandler, basic_auth, ca_bundle, running_upstream +from tests.unit.git_upstream import ( + ClosingAdapter, + HostRefused, + RefusingAdapter, + UpstreamHandler, + basic_auth, + ca_bundle, + running_upstream, +) TOKEN = "ghs_secrettoken" # Long enough that a relay that buffers fails the test instead of hanging it. @@ -410,18 +419,45 @@ def handler(h: UpstreamHandler) -> None: class TestAdapter: - def test_uses_the_adapter_it_is_given(self, upstream): - # Sentry passes an adapter that refuses internal addresses; the relay must send through it. - class HostRefused(Exception): + @pytest.mark.parametrize("refused_host_errors", [(), (LookupError,)], ids=["none", "another"]) + def test_uses_the_adapter_it_is_given(self, upstream, refused_host_errors): + # Sentry passes an adapter that refuses internal addresses; the relay must send through it. A refusal that + # refused_host_errors does not name reaches the caller as it was raised. + relay = GitRelay(adapter=RefusingAdapter, refused_host_errors=refused_host_errors, ca_bundle=ca_bundle()) + with pytest.raises(HostRefused, match="internal address"): + fetch_refs(relay, upstream) + assert upstream.connections == 0 + + def test_refused_host(self, upstream): + relay = GitRelay(adapter=RefusingAdapter, refused_host_errors=(HostRefused,), ca_bundle=ca_bundle()) + with pytest.raises(GitRemoteRefused) as exc_info: + fetch_refs(relay, upstream) + # The refusal passes through urllib3's frames, which hold the Authorization header. + assert_token_not_in(exc_info.value) + # The refusal names the address; the client has no use for it. + assert "127.0.0.1" not in exc_info.value.detail + assert upstream.connections == 0 + + def test_refused_host_named_as_a_requests_exception(self, upstream): + class HostRefusedConnectionError(requests.ConnectionError): pass - class RefusingAdapter(HTTPAdapter): + class Adapter(HTTPAdapter): def send(self, request, *args, **kwargs): - raise HostRefused(request.url) + raise HostRefusedConnectionError(request=request) - with pytest.raises(HostRefused): - fetch_refs(GitRelay(adapter=RefusingAdapter, ca_bundle=ca_bundle()), upstream) - assert upstream.requests == [] + relay = GitRelay(adapter=Adapter, refused_host_errors=(HostRefusedConnectionError,), ca_bundle=ca_bundle()) + with pytest.raises(GitRemoteRefused) as exc_info: + fetch_refs(relay, upstream) + assert_token_not_in(exc_info.value) + + @pytest.mark.parametrize( + "refused_host_errors", [HostRefused, [HostRefused], (HostRefused, "HostRefused")], ids=["class", "list", "str"] + ) + def test_refused_host_errors_must_be_a_tuple_of_exception_classes(self, refused_host_errors): + # In an except clause anything else fails only once a request does, as a TypeError chained to its error. + with pytest.raises(TypeError, match="tuple of exception classes"): + GitRelay(refused_host_errors=refused_host_errors) class TestEnvironment: diff --git a/tests/unit/test_git_server.py b/tests/unit/test_git_server.py index bdf6747..d0db302 100644 --- a/tests/unit/test_git_server.py +++ b/tests/unit/test_git_server.py @@ -17,7 +17,7 @@ from scm.types import GitRemote, Provider, Repository, RepositoryId from tests.unit.error_reports import assert_not_in_frames from tests.unit.git_builders import make_grant, make_provider, make_repository -from tests.unit.git_upstream import ClosingAdapter, basic_auth, running_upstream +from tests.unit.git_upstream import ClosingAdapter, HostRefused, RefusingAdapter, basic_auth, running_upstream SECRET = Secret("test-secret-0000000000000000000000") TOKEN = "ghs_secrettoken" @@ -400,6 +400,19 @@ def test_plain_http_remote(self): ("sentry.scm.git.request.failed", {"referrer": REFERRER, "code": "git_remote_not_https"}) ] + def test_adapter_refusing_the_host(self): + # e.g. Sentry's BlacklistAdapter refusing a GitHub Enterprise host that resolves to an internal address. + recorder = Recorder( + repository=make_repository(provider_name="github_enterprise", web_base_url="https://127.0.0.1:1") + ) + relay = GitRelay(adapter=RefusingAdapter, refused_host_errors=(HostRefused,)) + response = request(make_server(recorder, relay), signed_grant()) + assert_refused(response, 502, "at an address Sentry does not connect to") + assert recorder.counts == [ + ("sentry.scm.git.request.failed", {"referrer": REFERRER, "code": "git_remote_refused"}) + ] + assert recorder.error_codes == ["git_remote_refused"] + def test_provider_rejecting_the_token(self): # Passed on, a 401 would make git ask the sandboxed client for credentials. adapters: list[ClosingAdapter] = [] @@ -474,10 +487,8 @@ def test_no_refusal_is_a_401(self): assert 401 not in _STATUS.values() def test_other_errors_reach_the_caller(self): - # Sentry's view answers these the way it answers them elsewhere, e.g. a refused host as ApiHostError. - class HostRefused(Exception): - pass - + # Sentry's view answers these the way it answers them elsewhere, e.g. a refusal from an adapter whose error + # the relay was not told about. relay = MagicMock(spec=GitRelay) relay.forward.side_effect = HostRefused() recorder = Recorder() From c487605998b1ea46e12f81b3978f49501bcc5c5b Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Tue, 27 Oct 2026 23:51:08 +0100 Subject: [PATCH 2/2] feat(git): Let the relay's caller map adapter errors to coded errors A fixed list of refused-host exception types only covers that one case. A map_error callable lets the caller turn any error of its adapter into a coded error, while scm keeps owning the error classes and their status. The relay's own conversions of requests and urllib3 errors, and its own coded errors, still run first, so the mapper never sees an exception that holds the Authorization header and can't relabel the client going away. A mapped error is raised "from None" because a refusal raised as urllib3 connects carries frames that hold the header. --- src/scm/git/relay.py | 42 ++++++++++--------- src/scm/git/server.py | 5 +-- tests/unit/test_git_relay.py | 77 +++++++++++++++++++++++------------ tests/unit/test_git_server.py | 11 +++-- 4 files changed, 84 insertions(+), 51 deletions(-) diff --git a/src/scm/git/relay.py b/src/scm/git/relay.py index bd07aca..b14d4a5 100644 --- a/src/scm/git/relay.py +++ b/src/scm/git/relay.py @@ -11,7 +11,6 @@ ClientClosedRequest, GitOperationNotGranted, GitRemoteNotHttps, - GitRemoteRefused, GitRepositoryNotGranted, GitRouteNotSupported, ResourceBadGateway, @@ -19,6 +18,7 @@ ResourceGatewayTimeout, ResourceServiceUnavailable, RpcMalformedRequestHeaders, + SCMCodedError, ) from scm.git.types import GitOperation, Grant, Readable from scm.rpc.types import StreamResponse @@ -107,12 +107,16 @@ class GitRelay: ``adapter`` builds the adapter that sends each upstream request. Pass one that refuses internal addresses: a GitHub Enterprise or self-managed GitLab host is whatever the customer configured, and the relay connects wherever it resolves. Sentry's ``BlacklistAdapter`` does this for its API clients, and the class itself can be - passed. ``refused_host_errors`` names the exception classes the adapter raises for a refused host, such as - Sentry's ``RestrictedIPAddress``; the relay raises ``GitRemoteRefused`` in their place, which GitServer answers - like its other refusals. Any other error the adapter raises reaches the caller unchanged, unless it is a requests - or urllib3 exception, which the relay replaces like any other so that it cannot carry the token. The adapter - must not retry, because a retry cannot resend a body that has already been streamed. Without one, the relay uses - a plain ``HTTPAdapter``, which checks nothing. + passed. The adapter must not retry, because a retry cannot resend a body that has already been streamed. Without + one, the relay uses a plain ``HTTPAdapter``, which checks nothing. + + The relay replaces a requests or urllib3 exception from the upstream request with a coded error, so that it + cannot carry the token. ``map_error`` gets any other exception, such as the adapter's own for a refused host, + and returns either a coded error, which the relay raises in its place without the original, or None to let the + original reach the caller unchanged. GitServer answers a coded error like its other refusals. For an adapter + that refuses a host with ``HostRefused``:: + + GitRelay(adapter=..., map_error=lambda e: GitRemoteRefused() if isinstance(e, HostRefused) else None) The relay takes no settings from the environment. Through a proxy from ``HTTPS_PROXY``, the adapter would check the proxy's address rather than the provider's. ``ca_bundle`` names a CA file to trust instead of the @@ -123,19 +127,13 @@ def __init__( self, *, adapter: Callable[[], HTTPAdapter] = HTTPAdapter, - refused_host_errors: tuple[type[Exception], ...] = (), + map_error: Callable[[Exception], SCMCodedError | None] | None = None, ca_bundle: str | None = None, timeout: tuple[float, float] = DEFAULT_TIMEOUT, chunk_size: int = DEFAULT_CHUNK_SIZE, ) -> None: - # Checked now rather than when a request fails: in an except clause, anything but a tuple of exception - # classes turns every failed request into a TypeError chained to the original error. - if not isinstance(refused_host_errors, tuple) or not all( - isinstance(error, type) and issubclass(error, Exception) for error in refused_host_errors - ): - raise TypeError("refused_host_errors must be a tuple of exception classes.") self.adapter = adapter - self.refused_host_errors = refused_host_errors + self.map_error = map_error self.ca_bundle = ca_bundle self.timeout = timeout self.chunk_size = chunk_size @@ -213,10 +211,6 @@ def forward( allow_redirects=False, timeout=self.timeout, ) - # First, in case the adapter refuses with a requests exception. "from None" drops the refusal: raised as the - # adapter connects, its traceback runs through urllib3's frames, which hold the Authorization header. - except self.refused_host_errors: - raise GitRemoteRefused() from None # "from None" drops requests' exception, whose request holds the Authorization header. except requests.Timeout: raise ResourceGatewayTimeout() from None @@ -231,6 +225,16 @@ def forward( # Any other failure of the request, e.g. a malformed response. A requests exception, like those above, # holds the request. raise ResourceBadGateway() from None + # The relay's own, such as the client's connection failing while its body is sent. + except SCMCodedError: + raise + except Exception as error: + mapped = self.map_error(error) if self.map_error is not None else None + if mapped is None: + raise + # "from None" drops the original. Raised as the adapter connects, e.g. for a refused host, its traceback + # runs through urllib3's frames, which hold the Authorization header. + raise mapped from None finally: # Once a response arrives, _stream closes the session when the response ends. if upstream is None: diff --git a/src/scm/git/server.py b/src/scm/git/server.py index f2c5ced..f385b1b 100644 --- a/src/scm/git/server.py +++ b/src/scm/git/server.py @@ -60,9 +60,8 @@ class GitServer: when it failed, as tags: ``repository_id`` as requested, then ``organization_id`` and ``referrer`` once the grant is verified, ``operation``, ``provider`` and ``remote_host``. A tag is missing when the request failed before it was known. A client closing its request is not reported, since there is nothing to fix. Any other - exception, such as the token supplier failing, or the relay's adapter refusing a host with an error its - ``refused_host_errors`` does not name, reaches the caller unchanged, so the caller can answer it the way it - answers the same error elsewhere. + exception, such as the token supplier failing, or an error of the relay's adapter that its ``map_error`` leaves + alone, reaches the caller unchanged, so the caller can answer it the way it answers the same error elsewhere. Every count is tagged with the grant's ``referrer`` once the grant is verified, so Sentry can tell a coding job's clone from Seer's own fetch or push. diff --git a/tests/unit/test_git_relay.py b/tests/unit/test_git_relay.py index d7fe48c..7783f56 100644 --- a/tests/unit/test_git_relay.py +++ b/tests/unit/test_git_relay.py @@ -3,7 +3,7 @@ import socket import threading import time -from collections.abc import Iterator +from collections.abc import Callable, Iterator from unittest.mock import MagicMock import pytest @@ -419,45 +419,72 @@ def handler(h: UpstreamHandler) -> None: class TestAdapter: - @pytest.mark.parametrize("refused_host_errors", [(), (LookupError,)], ids=["none", "another"]) - def test_uses_the_adapter_it_is_given(self, upstream, refused_host_errors): - # Sentry passes an adapter that refuses internal addresses; the relay must send through it. A refusal that - # refused_host_errors does not name reaches the caller as it was raised. - relay = GitRelay(adapter=RefusingAdapter, refused_host_errors=refused_host_errors, ca_bundle=ca_bundle()) + def test_uses_the_adapter_it_is_given(self, upstream): + # Sentry passes an adapter that refuses internal addresses; the relay must send through it. with pytest.raises(HostRefused, match="internal address"): - fetch_refs(relay, upstream) + fetch_refs(GitRelay(adapter=RefusingAdapter, ca_bundle=ca_bundle()), upstream) assert upstream.connections == 0 - def test_refused_host(self, upstream): - relay = GitRelay(adapter=RefusingAdapter, refused_host_errors=(HostRefused,), ca_bundle=ca_bundle()) + +def refusal_mapper(seen: list[Exception]) -> Callable[[Exception], SCMCodedError | None]: + """A map_error that records each error it gets and maps HostRefused, the way Sentry maps its refusal.""" + + def map_error(error: Exception) -> SCMCodedError | None: + seen.append(error) + return GitRemoteRefused() if isinstance(error, HostRefused) else None + + return map_error + + +class TestMapError: + def test_mapped_error_is_raised_in_place_of_the_original(self, upstream): + seen: list[Exception] = [] + relay = GitRelay(adapter=RefusingAdapter, map_error=refusal_mapper(seen), ca_bundle=ca_bundle()) with pytest.raises(GitRemoteRefused) as exc_info: fetch_refs(relay, upstream) + assert [type(error) for error in seen] == [HostRefused] # The refusal passes through urllib3's frames, which hold the Authorization header. assert_token_not_in(exc_info.value) # The refusal names the address; the client has no use for it. assert "127.0.0.1" not in exc_info.value.detail - assert upstream.connections == 0 - def test_refused_host_named_as_a_requests_exception(self, upstream): - class HostRefusedConnectionError(requests.ConnectionError): - pass + @pytest.mark.parametrize("map_error", [None, lambda error: None], ids=["no_mapper", "unmapped"]) + def test_unmapped_error_reaches_the_caller_unchanged(self, upstream, map_error): + with pytest.raises(HostRefused, match="internal address"): + fetch_refs(GitRelay(adapter=RefusingAdapter, map_error=map_error, ca_bundle=ca_bundle()), upstream) - class Adapter(HTTPAdapter): + @pytest.mark.parametrize( + ("make_error", "expected"), + [ + pytest.param( + lambda request: requests.ConnectionError("refused", request=request), + ResourceServiceUnavailable, + id="requests", + ), + pytest.param( + lambda request: urllib3.exceptions.HTTPError("bad response"), ResourceBadGateway, id="urllib3" + ), + ], + ) + def test_requests_and_urllib3_errors_never_reach_the_mapper(self, upstream, make_error, expected): + # A requests exception holds the request and its Authorization header. + class FailingAdapter(HTTPAdapter): def send(self, request, *args, **kwargs): - raise HostRefusedConnectionError(request=request) + raise make_error(request) - relay = GitRelay(adapter=Adapter, refused_host_errors=(HostRefusedConnectionError,), ca_bundle=ca_bundle()) - with pytest.raises(GitRemoteRefused) as exc_info: + seen: list[Exception] = [] + relay = GitRelay(adapter=FailingAdapter, map_error=refusal_mapper(seen), ca_bundle=ca_bundle()) + with pytest.raises(expected): fetch_refs(relay, upstream) - assert_token_not_in(exc_info.value) + assert seen == [] - @pytest.mark.parametrize( - "refused_host_errors", [HostRefused, [HostRefused], (HostRefused, "HostRefused")], ids=["class", "list", "str"] - ) - def test_refused_host_errors_must_be_a_tuple_of_exception_classes(self, refused_host_errors): - # In an except clause anything else fails only once a request does, as a TypeError chained to its error. - with pytest.raises(TypeError, match="tuple of exception classes"): - GitRelay(refused_host_errors=refused_host_errors) + def test_the_relays_own_errors_never_reach_the_mapper(self, upstream): + seen: list[Exception] = [] + relay = GitRelay(map_error=refusal_mapper(seen), ca_bundle=ca_bundle()) + with pytest.raises(SCMCodedError) as exc_info: + upload_pack(relay, upstream, io.BytesIO(b"00"), headers={"Content-Length": "4"}) + assert exc_info.value.code == "client_closed_request" + assert seen == [] class TestEnvironment: diff --git a/tests/unit/test_git_server.py b/tests/unit/test_git_server.py index d0db302..60f63b9 100644 --- a/tests/unit/test_git_server.py +++ b/tests/unit/test_git_server.py @@ -7,7 +7,7 @@ import pytest -from scm.errors import ClientClosedRequest, RateLimitExceeded +from scm.errors import ClientClosedRequest, GitRemoteRefused, RateLimitExceeded from scm.git.grant import sign_grant from scm.git.relay import GitRelay, RelayResponse, ResponseBody from scm.git.server import _STATUS, GitServer @@ -405,7 +405,10 @@ def test_adapter_refusing_the_host(self): recorder = Recorder( repository=make_repository(provider_name="github_enterprise", web_base_url="https://127.0.0.1:1") ) - relay = GitRelay(adapter=RefusingAdapter, refused_host_errors=(HostRefused,)) + relay = GitRelay( + adapter=RefusingAdapter, + map_error=lambda error: GitRemoteRefused() if isinstance(error, HostRefused) else None, + ) response = request(make_server(recorder, relay), signed_grant()) assert_refused(response, 502, "at an address Sentry does not connect to") assert recorder.counts == [ @@ -487,8 +490,8 @@ def test_no_refusal_is_a_401(self): assert 401 not in _STATUS.values() def test_other_errors_reach_the_caller(self): - # Sentry's view answers these the way it answers them elsewhere, e.g. a refusal from an adapter whose error - # the relay was not told about. + # Sentry's view answers these the way it answers them elsewhere, e.g. an adapter's error that the relay's + # map_error leaves alone. relay = MagicMock(spec=GitRelay) relay.forward.side_effect = HostRefused() recorder = Recorder()