Skip to content

fix(data-imports): keep known-transient S3 blips out of error tracking in get_delta_table - #74653

Merged
trunk-io[bot] merged 1 commit into
masterfrom
posthog-code/fix-transient-s3-noise-get-delta-table
Jul 29, 2026
Merged

fix(data-imports): keep known-transient S3 blips out of error tracking in get_delta_table#74653
trunk-io[bot] merged 1 commit into
masterfrom
posthog-code/fix-transient-s3-noise-get-delta-table

Conversation

@Gilbert09

Copy link
Copy Markdown
Member

Problem

Error tracking surfaced an OSError: Generic S3 error ... operation timed out from DeltaTableHelper.get_delta_table(), raised while calling deltalake.DeltaTable.is_deltatable(...).

This is the same class of self-recovering S3 blip already documented by TRANSIENT_OBJECT_STORE_ERRORS/is_transient_object_store_error in delta_table_helper.py — a transient network hiccup on an S3 LIST request, not a customer credential problem or a bug in our code. It's already retryable today: nothing here is in any source's NonRetryableErrors, and Temporal's default activity retry policy applies, so the sync itself was never at risk. The gap is purely reporting noise: get_delta_table()'s two except blocks (around the is_deltatable existence check and the DeltaTable() open) call capture_exception unconditionally, without ever consulting the classifier that already exists in the same file for exactly this error family.

Changes

  • Added DeltaTableHelper._capture_unless_transient, which calls capture_exception only when the error is not a known-transient object-store blip (falls back to a warning log otherwise). The exception is always re-raised regardless, so Temporal's retry behavior is unchanged.
  • Wired both except blocks in get_delta_table() through it.

How did you test this code?

  • Extended TestGetDeltaTableUnrecoverableErrors in test_delta_table_helper.py:
    • Adjusted the existing is_deltatable capture test to use a genuinely non-transient error (Access Denied), since its previous example text happened to match the transient-error substring pattern.
    • Added test_is_deltatable_transient_error_is_not_captured_but_still_reraised, reproducing the exact production error text — asserts capture_exception is not called, a warning is logged, and the exception still propagates (so Temporal still retries).
  • Ran the affected test class locally with the object-store network call mocked out (this sandbox has no local S3-compatible service to hit ensure_bucket_exists against, same pre-existing constraint as the rest of this test file); both new/updated cases pass with the expected behavior.
  • uv run mypy --cache-fine-grained . (repo-wide) — clean.
  • ruff check --fix / ruff format — clean.

Automatic notifications

  • Publish to changelog?
  • Alert Sales and Marketing teams?

Docs update

N/A — internal error-classification 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 stack trace pinning the failure to get_delta_table's is_deltatable call. Traced the call path and confirmed via a research subagent that Temporal's retry policy already treats this OSError as retryable — the actual gap was reporting noise, not retry correctness.

Checked for duplicate open PRs by exception type, message phrase, and module path, and scanned the maintainer's own open PR queue. Found several related-but-non-overlapping PRs touching transient S3 handling elsewhere in the same file/module (e.g. #74388 for the _handle_import_error fallback path, #69306 and #74630 for vacuum/compact maintenance, #73395 for query-folder cleanup) — none of them touch get_delta_table's own capture_exception calls, which is the gap this PR closes.

Invoked /writing-tests before adding the regression tests.

…g in get_delta_table

get_delta_table() unconditionally reported every exception from the is_deltatable check and the DeltaTable() open to error tracking, even ones matching the existing is_transient_object_store_error classifier (e.g. an S3 LIST request timing out). Those are self-recovering network blips that Temporal already retries automatically, so reporting them is just noise.

Skip capture_exception (log a warning instead) when the error matches the known-transient pattern, in both except blocks; the exception is still re-raised either way so retry behavior is unchanged.

Generated-By: PostHog Code
Task-Id: e7f0cc1d-199a-486f-b740-0163f18a42e7
@trunk-io

trunk-io Bot commented Jul 29, 2026

Copy link
Copy Markdown

😎 Merged successfully - details.

@github-actions

Copy link
Copy Markdown
Contributor

Hey @Gilbert09! 👋

It looks like your git author email on this PR isn't your @posthog.com address (owerstom@gmail.com). Since you're on the PostHog team, it's worth pointing your local git author email at your @posthog.com address. Why it matters:

  • Consistent work identity in git history — internal tooling that attributes commits to team members keys off your @posthog.com address.
  • Keeps team contributions easy to tell apart from external community ones when scanning history.

You can fix it for this repo with:

git config user.email "you@posthog.com"

Or set it globally with git config --global user.email "you@posthog.com". No need to redo this PR — just a nudge for next time. 🙂

@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team July 29, 2026 12:09
@Gilbert09 Gilbert09 added the stamphog Request AI approval (no full review) label Jul 29, 2026 — with PostHog

@stamphog stamphog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small, contained fix that suppresses error-tracking noise for known-transient S3 errors while always re-raising (no change to retry behavior); no risky territory touched, author owns the code with strong familiarity, and tests cover both branches.

  • Author wrote 83% of the modified lines and has 0 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 15L, 1F substantive, 46L/2F incl. docs/generated/snapshots — within ceiling
tier T1-agent / T1b-small (46L, 2F, single-area, fix)
stamphog 2.0.0b3 .stamphog/policy.yml @ 1bad8d1 · reviewed head 0357872

@talyn-app

talyn-app Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

/trunk merge

@trunk-io
trunk-io Bot merged commit 3f38956 into master Jul 29, 2026
367 checks passed
@trunk-io
trunk-io Bot deleted the posthog-code/fix-transient-s3-noise-get-delta-table branch July 29, 2026 13:39
@deployment-status-posthog

deployment-status-posthog Bot commented Jul 29, 2026

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-07-29 14:33 UTC Run
prod-us ✅ Deployed 2026-07-29 14:46 UTC Run
prod-eu ✅ Deployed 2026-07-29 14:49 UTC Run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stamphog Request AI approval (no full review)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant