From 1893fb57ab7e81f2cc5d3222c4ca6945b90c2999 Mon Sep 17 00:00:00 2001 From: Tom Owers Date: Wed, 29 Jul 2026 12:46:43 +0100 Subject: [PATCH] fix(data-imports): keep exhausted REST retries out of error tracking RESTClientRetryableError already carries `NonReportableError`'s sibling type contract in spirit (`_handle_import_error` logs it at warning, not exception), but the activity interceptor that decides whether to call `capture_exception` only skips reporting for exception types that actually subclass `NonReportableError`. Log level never reached that check, so a transient 429/5xx/timeout that survives all 5 tenacity attempts still minted an error-tracking issue on every occurrence - e.g. a Cloudflare HTTP 524 timeout on a Cal.com sync. Make `RESTClientRetryableError` subclass `NonReportableError`, the same way `RESTClientNonRetryableError` already does, so the activity interceptor actually honors the "benign, self-recovering" classification these errors were always meant to have. Temporal still retries the whole activity as before - this only changes whether the escaped exception is reported. branch: posthog-code/fix-rest-retryable-error-tracking-noise Generated-By: PostHog Code Task-Id: 3eff308d-cef3-4ca5-8c9e-9b42407c2459 --- .../sources/common/rest_source/rest_client.py | 9 ++++++++- .../sources/common/rest_source/tests/test_rest_client.py | 6 +++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/products/warehouse_sources/backend/temporal/data_imports/sources/common/rest_source/rest_client.py b/products/warehouse_sources/backend/temporal/data_imports/sources/common/rest_source/rest_client.py index e5f92e28f622..a93bd7f3dbc3 100644 --- a/products/warehouse_sources/backend/temporal/data_imports/sources/common/rest_source/rest_client.py +++ b/products/warehouse_sources/backend/temporal/data_imports/sources/common/rest_source/rest_client.py @@ -30,7 +30,14 @@ logger = logging.getLogger(__name__) -class RESTClientRetryableError(Exception): +class RESTClientRetryableError(NonReportableError): + """A transient failure (429/5xx, connection reset/timeout, malformed/truncated body) that + tenacity reissues up to the client's attempt cap. Once that budget is exhausted it escapes to + the activity and Temporal's own activity retry takes over — the upstream blip is expected to + clear, not a PostHog defect. Subclasses NonReportableError, like ``RESTClientNonRetryableError``, + so the activity interceptor keeps it out of error tracking instead of minting an issue per blip. + """ + def __init__(self, message: str, retry_after: Optional[float] = None) -> None: super().__init__(message) self.retry_after = retry_after diff --git a/products/warehouse_sources/backend/temporal/data_imports/sources/common/rest_source/tests/test_rest_client.py b/products/warehouse_sources/backend/temporal/data_imports/sources/common/rest_source/tests/test_rest_client.py index 4b1126306711..b75c81e62014 100644 --- a/products/warehouse_sources/backend/temporal/data_imports/sources/common/rest_source/tests/test_rest_client.py +++ b/products/warehouse_sources/backend/temporal/data_imports/sources/common/rest_source/tests/test_rest_client.py @@ -353,10 +353,14 @@ def test_send_request_raises_retryable_after_persistent_transient_error( mock_session.send.return_value = error client = RESTClient(base_url="https://api.example.com") - with pytest.raises(RESTClientRetryableError): + with pytest.raises(RESTClientRetryableError) as ctx: list(client.paginate(path="/items", paginator=SinglePagePaginator())) assert mock_session.send.call_count == 5 + # An upstream blip surviving every tenacity attempt is expected to clear on Temporal's own + # activity retry, not a PostHog defect, so it must carry the non-reportable marker the + # activity interceptor uses to keep it out of error tracking. + assert isinstance(ctx.value, NonReportableError) @pytest.mark.parametrize( "content",