fix(data-imports): keep source-classified retryable errors out of error tracking - #74744
Merged
trunk-io[bot] merged 1 commit intoJul 29, 2026
Merged
Conversation
…or tracking A source's `get_retryable_errors()` (e.g. MySQL's "Lost connection to MySQL server during query") is meant to keep a transient, self-recovering error out of error tracking once it escapes to `_handle_import_error`. The handler logged it at `warning` instead of `exception`, but that only affects our own logs — the Temporal activity interceptor reports whatever exception type escapes the activity, regardless of log level, unless it's a `NonReportableError`. Re-raising the bare exception meant these errors landed in error tracking anyway. Re-raise as `NonReportableError` (chained via `from error`) instead, mirroring how `RESTClientRetryableError` already does this for REST sources. Generated-By: PostHog Code Task-Id: d79e0d52-59ff-4e1f-9f4c-4dedc17c7faa
|
😎 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 |
There was a problem hiding this comment.
Small, contained fix in warehouse-sources error handling (not risky territory) by an owning-team author with strong familiarity; the diff matches the description and the NonReportableError mechanism it relies on checks out in the codebase.
- Author wrote 83% of the modified lines and has 24 merged PRs in these paths (familiarity STRONG).
- 👍 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 | ✓ | 12L, 1F substantive, 23L/2F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1b-small (23L, 2F, single-area, fix) |
| stamphog 2.0.0b3 | .stamphog/policy.yml @ 30ad54b · reviewed head b7d0c5d |
Contributor
|
/trunk merge |
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
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
Error tracking surfaced an
OperationalError: (2013, 'Lost connection to MySQL server during query (The read operation timed out)')from the MySQL data-warehouse import source, raised from the streaming row-fetch loop inmysql.py's_stream_with_optional_force_index.This is exactly the class of error MySQL's
get_retryable_errors()already lists ("Lost connection to MySQL server during query"), specifically so it stays out of error tracking as benign, self-recovering noise — Temporal retries the whole activity and the connection drop resolves itself. But it still got reported.The gap is in the shared
_handle_import_error(import_data_sync.py): when an error matches a source'sget_retryable_errors(), it logs atwarning(notexception) and re-raises the original exception. Logging atwarningonly affects our own structured logs — the Temporal activity interceptor (posthog/temporal/common/posthog_client.py) reports whatever exception type escapes the activity to error tracking regardless of log level. It only skips reporting forNonReportableError(and a couple of other known-benign marker types). Since the message-matched retryable path re-raised the bare exception instead of aNonReportableError, it was captured anyway — silently defeating the intent already documented in this function's docstring and inMySQLSource.get_non_retryable_errors's siblingget_retryable_errors.This affects every source that implements
get_retryable_errors()(MySQL, Postgres, Snowflake, Salesforce, Mixpanel, HubSpot, and others), not just MySQL.Nothing here needed adding to
NonRetryableErrors— the failure is transient infrastructure (a read timeout mid-stream) and Temporal's retry already handles it correctly; the only problem was the error-tracking noise.Changes
_handle_import_errornow re-raises a source-classified retryable error asNonReportableError(error_msg) from errorinstead of the bare exception, so Temporal still retries the activity as usual but the interceptor no longer reports it. Mirrors howRESTClientRetryableError(already aNonReportableErrorsubclass) achieves this for REST-based sources.warningalone kept these out of error tracking.How did you test this code?
test_source_classified_retryable_error_logged_as_warning_not_exceptionintest_import_data_sync.pyto assert the re-raised exception is aNonReportableError(not just anyException) with the original error chained via__cause__— this is the regression guard: reverting toraise errorwould fail this assertion even though the log-level assertions would still pass.pytest products/warehouse_sources/backend/temporal/data_imports/workflow_activities/tests/test_import_data_sync.py(12 passed) and the MySQL source's retryable/non-retryable classification tests (35 passed).ruff check/ruff format --checkon the changed files: clean.mypy --cache-fine-grainedtargeted at the changed file: clean. A repo-wide mypy run (as CI runs it) was still in progress when this PR was opened; will follow up if it surfaces anything.Automatic notifications
Docs update
N/A — internal error-classification/reporting change, no user-facing or API behavior change.
🤖 Agent context
Autonomy: Fully autonomous
Triaged directly from the linked error-tracking issue via PostHog's error-tracking MCP tools (
query-error-tracking-issue,query-error-tracking-issue-events), which surfaced the full stack trace confirming the failure genuinely originated inmysql.py. Traced the call path from the MySQL streaming cursor throughget_rows/_handle_import_error/import_data_activity_syncup to the Temporal activity interceptor, and readposthog/temporal/common/posthog_client.pyto confirm it captures by exception type, not log level.Checked for duplicate open PRs by exception type, message phrase, and module path, and scanned the maintainer's own open PR queue. Found one closely related PR, #74388 ("keep transient S3 credential blips out of error tracking"), which fixes the same class of interceptor-reporting gap but for a different, non-overlapping branch of
_handle_import_error(a newis_transient_object_store_errorcheck) — it doesn't touch theget_retryable_errors()branch this PR fixes.Invoked
/writing-testsbefore extending the regression test.