diff --git a/tests/client/client_tests.py b/tests/client/client_tests.py index 05388921f..e73e726ab 100644 --- a/tests/client/client_tests.py +++ b/tests/client/client_tests.py @@ -274,33 +274,44 @@ def test_send_remote_failover_sync(should_try, sending_elasticapm_client, caplog assert not sending_elasticapm_client._transport.state.did_fail() -@mock.patch("elasticapm.transport.http.Transport.send") -@mock.patch("elasticapm.transport.base.TransportState.should_try") -def test_send_remote_failover_sync_non_transport_exception_error(should_try, http_send, caplog): - should_try.return_value = True - +@mock.patch("elasticapm.transport.base.TransportState.should_try", return_value=True) +def test_send_remote_failover_sync_non_transport_exception_error(should_try, caplog): client = Client( server_url="http://example.com", service_name="app_name", secret_token="secret", - transport_class="elasticapm.transport.http.Transport", + transport_class="tests.fixtures.MockSendHTTPTransport", metrics_interval="0ms", metrics_sets=[], ) + # test error - http_send.side_effect = ValueError("oopsie") + client._transport.send_mock.side_effect = ValueError("oopsie") with caplog.at_level("ERROR", "elasticapm.transport"): client.capture_message("foo", handled=False) - client._transport.flush() + try: + client._transport.flush() + except ValueError: + # give flush a bit more room because we may take a bit more than the max timeout to flush + client._transport._flushed.wait(timeout=1) assert client._transport.state.did_fail() assert_any_record_contains(caplog.records, "oopsie", "elasticapm.transport") # test recovery - http_send.side_effect = None + client._transport.send_mock.side_effect = None client.capture_message("foo", handled=False) - client.close() + try: + client._transport.flush() + except ValueError: + # give flush a bit more room because we may take a bit more than the max timeout to flush + client._transport._flushed.wait(timeout=1) + # We have a race here with the queue where we would end up checking for did_fail before the message + # is being handled by the queue, so sleep a bit and retry to give it enough time + retries = 0 + while client._transport.state.did_fail() and retries < 3: + time.sleep(0.1) + retries += 1 assert not client._transport.state.did_fail() - client.close() @pytest.mark.parametrize("validating_httpserver", [{"skip_validate": True}], indirect=True) diff --git a/tests/fixtures.py b/tests/fixtures.py index 9d69d80cb..1b9119b99 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -59,6 +59,7 @@ from elasticapm.conf.constants import SPAN from elasticapm.instrumentation import register from elasticapm.traces import execution_context +from elasticapm.transport.http import Transport from elasticapm.transport.http_base import HTTPTransportBase from elasticapm.utils.threading import ThreadManager @@ -396,6 +397,18 @@ def get_config(self, current_version=None, keys=None): return False, None, 30 +class MockSendHTTPTransport(Transport): + """Mocking the send method of the Transport class sometimes fails silently in client tests. + After spending some time trying to understand this with no luck just use this class instead.""" + + def __init__(self, url, *args, **kwargs): + self.send_mock = mock.Mock() + super().__init__(url, *args, **kwargs) + + def send(self, data, forced_flush=False, custom_url=None, custom_headers=None): + return self.send_mock(data, forced_flush, custom_url, custom_headers) + + class TempStoreClient(Client): def __init__(self, config=None, **inline) -> None: inline.setdefault("transport_class", "tests.fixtures.DummyTransport")