Skip to content

fix(warehouse-sources): stop one unreadable HubSpot object failing the properties table - #95236

Closed
posthog[bot] wants to merge 1 commit into
masterfrom
posthog/hubspot-properties-skip-forbidden
Closed

fix(warehouse-sources): stop one unreadable HubSpot object failing the properties table#95236
posthog[bot] wants to merge 1 commit into
masterfrom
posthog/hubspot-properties-skip-forbidden

Conversation

@posthog

@posthog posthog Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Problem

A HubSpot source's properties table stops syncing entirely — status Failed, zero rows, never completes — for any portal whose grant cannot read one of HubSpot's object types. Every other table on the same source syncs fine, and the customer has no way to opt out: get_properties_rows fans out over every entry in HUBSPOT_ENDPOINTS, so the table ignores which tables were selected for sync.

The connector already intends to tolerate this. _iter_pages has a _SKIPPABLE_STATUSES = (403, 404) guard whose comment says "Skipping keeps one unavailable object type from failing the table." The guard catches requests.HTTPError. Since raise_for_hubspot_status was introduced, a 403 is raised as HubspotForbiddenError, which subclasses NonReportableError(Exception) — not requests.HTTPError. So the 403 half of that guard has been unreachable, and one unreadable object type takes the whole table down.

The customer-facing error tells them to reconnect HubSpot, which does not help: the grant is fine for everything they selected, and reconnecting cannot make an object readable that their plan does not include.

Changes

  • _iter_pages now also skips HubspotForbiddenError, so an object type the portal cannot read is logged and passed over instead of failing the properties, pipelines, pipeline_stages, and owners tables.
  • This covers the HubspotMissingScopeError subclass too, so a scope-gated object such as leads no longer fails the properties table on connections that were never granted its optional scope.
  • 404 handling and the "a 5xx still fails the table" behavior are unchanged.

How did you test this code?

test_properties_skips_an_object_type_the_portal_cannot_read previously raised requests.HTTPError directly from a patched fetch_data, so it kept passing against an exception class the fetch path no longer raises — that mismatch is why this shipped. It now raises through the real raise_for_hubspot_status, so the skip is exercised against whatever the production path actually produces. Because it lets every non-deals path 403, it covers HubspotMissingScopeError (via leads) as well as the plain forbidden case.

Verified the test fails on master and passes with the fix:

uv run pytest products/warehouse_sources/backend/temporal/data_imports/sources/hubspot/test/test_hubspot_metadata.py
  • Without the metadata change: test_properties_skips_an_object_type_the_portal_cannot_read fails with HubspotForbiddenError: 403 Client Error: Forbidden for url: .../crm/properties/2026-03/contacts.
  • With it: 9 passed.

ruff format --check, ruff check, and mypy are clean on the changed files. No manual end-to-end run against a real HubSpot portal was done.

Automatic notifications

  • Publish to changelog?

Docs update

None.

🤖 Agent context

Autonomy: Fully autonomous

Written by Claude Code during warehouse-sources ticket triage, following the repo's triaging-warehouse-sync-tickets skill and the ticket-investigation-warehouse-sources skill from the PostHog skills store. The bug was found by reading metadata.py, helpers.py, and scopes.py, then confirmed against production sync logs for an affected source.

Two fixes were considered. Giving the unreadable object a required_scope was rejected: HubspotMissingScopeError subclasses HubspotForbiddenError, so the properties job would still escape the guard and fail, just with different wording. Catching the exception type is what actually restores the intended skip, and it fixes the scope-gated case at the same time.

No duplicate: gh pr list --state open --search across "hubspot", "403 OR forbidden", and the module path found nothing touching this guard. The nearest neighbors — #91331 (HubSpot 401 classification, source.py) and #91423 (Decagon/Northpass 403 messaging) — share no root cause or files.

Public artifact: the investigation drew on a customer support ticket and that customer's production sync logs. Nothing from either appears in this PR. The test fixtures are the ones already in the file, and the description describes the failure generically.

…e properties table

The properties lookup table fans out over every HubSpot object endpoint, and its
guard skipped an object the portal cannot read by catching `requests.HTTPError`.
Since 403s started being raised as `HubspotForbiddenError` (a `NonReportableError`,
not an `HTTPError`), that guard no longer fires on 403 and a single unreadable
object type fails the whole table.

Catch `HubspotForbiddenError` too, which also covers the `HubspotMissingScopeError`
subclass raised for scope-gated objects.

The existing regression test injected `requests.HTTPError` directly, so it kept
passing against an exception class the fetch path no longer raises. It now raises
through `raise_for_hubspot_status`, so the skip is exercised against whatever the
production path actually produces.

Generated-By: PostHog Desktop
Task-Id: 27f98539-efe2-40b1-a44a-780543bd4e58
@trunk-io

trunk-io Bot commented Sep 4, 2026

Copy link
Copy Markdown

Merging to master in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team September 4, 2026 13:57
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

🤖 CI report

⚠️ Trunk lane — backend Python lane

This PR is assigned to the backend Python lane. It runs backend Python tests and may merge in parallel with PRs in other lanes.

@trunk-io

trunk-io Bot commented Sep 4, 2026

Copy link
Copy Markdown

Static BadgeStatic BadgeStatic Badge

View Full Report ↗︎Docs

@danielcarletti

Copy link
Copy Markdown
Contributor

Closing in favor of #95301, which fixes the same root cause.

The two differ on one decision. This PR skips the 403 for every lookup table, owners included, so a portal that cannot read owners syncs an empty table and reports success. #95301 keeps owners failing, so the customer still gets the reconnect message. That is the outcome the originating support case supports: the reporter's owners problem resolved once they reconnected, and the error is what told them to.

#95301 also covers pipelines and pipeline_stages, which this PR leaves on the broken path, and it patches the HTTP session rather than calling raise_for_hubspot_status inside a patched fetch_data, so the test exercises the real request path.

The root-cause analysis here was correct, and #95301 builds on it.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant