fix(data-imports): keep exhausted REST retries out of error tracking - #74640
Merged
trunk-io[bot] merged 1 commit intoJul 29, 2026
Merged
Conversation
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
|
😎 Merged successfully - details. |
Contributor
|
Hey @Gilbert09! 👋 It looks like your git author email on this PR isn't your
You can fix it for this repo with: git config user.email "you@posthog.com"Or set it globally with |
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Trivial, well-tested reclassification of an already-existing exception to suppress error-tracking noise for transient retryable failures; author is on the owning team and diff matches the description exactly.
- Author wrote 0% of the modified lines and has 15 merged PRs in these paths (familiarity MODERATE).
- 👍 on the PR from hex-security-app[bot].
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 9L, 1F substantive, 15L/2F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1a-trivial (15L, 2F, single-area, fix) |
| stamphog 2.0.0b3 | .stamphog/policy.yml @ 545937f · reviewed head 1893fb5 |
Contributor
|
/trunk merge |
trunk-io
Bot
deleted the
posthog-code/fix-rest-retryable-error-tracking-noise
branch
July 29, 2026 13:02
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
An error-tracking issue fired for a data warehouse import sync:
RESTClientRetryableError: HTTP 524 for https://api.cal.com/v2/bookings(error tracking issue). HTTP 524 is Cloudflare's "origin didn't respond in time" timeout - a transient upstream blip, not a bug.The shared REST engine already retries this internally (tenacity, 5 attempts with backoff) before giving up, and
import_data_sync.py's_handle_import_erroralready classifiesRESTClientRetryableErroras "benign, self-recovering" and logs it atwarninginstead ofexceptionspecifically so it wouldn't show up as error-tracking noise. That intent doesn't hold: the Temporal activity interceptor that decides whether to callcapture_exception(posthog/temporal/common/posthog_client.py) only skips reporting for exceptions that subclassNonReportableError- the application log level has no effect on it. So every exhausted-retry transient error from any REST-based source (429s, 5xx, connection resets/timeouts) still mints a fresh error-tracking issue, once its retry budget runs out, on every occurrence.RESTClientNonRetryableError(the sibling "retrying can never help" error) already solves this correctly by subclassingNonReportableError.RESTClientRetryableErrordidn't.Changes
RESTClientRetryableErrornow subclassesNonReportableError, the same patternRESTClientNonRetryableErroralready uses, so the activity interceptor actually keeps it out of error tracking once it escapes the REST client's own retry loop.How did you test this code?
test_send_request_raises_retryable_after_persistent_transient_errorintest_rest_client.pyto assert the raisedRESTClientRetryableErroris also aNonReportableError, mirroring the existing assertion forRESTClientNonRetryableErrorintest_non_json_response_body_is_non_retryable.uv run pytest products/warehouse_sources/backend/temporal/data_imports/sources/common/rest_source/tests/test_rest_client.py- 34 passed.uv run pytest products/warehouse_sources/backend/temporal/data_imports/workflow_activities/tests/test_import_data_sync.py- 12 passed (unaffected, confirms_handle_import_error's type-basedRESTClientRetryableErrorbranch still behaves as before).uv run mypy --cache-fine-grained .- no issues in 17074 source files.uv run ruff check/ruff format --checkon changed files - clean.Automatic notifications
Docs update
N/A - internal error-tracking classification, no user-facing behavior or API change.
🤖 Agent context
Autonomy: Fully autonomous
capture_exception) for a different error class (transient S3/object-store errors), confirming this is an established pattern rather than a novel fix - it doesn't touchRESTClientRetryableErroror this file, so no duplicate.RESTClientNonRetryableErrorprecedent in the same file.