[8.19] [GitHub] Propagate fetch errors instead of swallowing them (#4119) - #4136
Merged
Merged
Conversation
) Backport of #4119 to 8.19. The GitHub connector previously wrapped each fetch method in `try/except Exception: log warning`, so a sync could "succeed" while syncing nothing. This ports the two-tier error handling, removes broad exception handling, and hardens null-handling of GraphQL responses. - Page/API-level failures now propagate out of _fetch_repos / _fetch_pull_requests / _fetch_issues / _fetch_files / _fetch_files_by_path, failing the sync when data can't be fetched. - Per-document enrichment failures are WARN-logged and skipped via narrow gidgethub.GitHubException guards. - Add RateLimitingError; _put_to_sleep now only sleeps and its callers re-raise RateLimitingError so the @retryable layer retries it. - Replace bare `except Exception` with gidgethub.GitHubException (plus the connector's own Unauthorized/Forbidden exceptions in log-and-reraise spots like ping and _get_invalid_repos_for_personal_access_token). - Switch connection-field access to the `... or {}` / `... or []` idiom so null GraphQL connection fields no longer abort the sync. Ported to the single-module 8.19 layout (connectors/sources/github.py). Co-authored-by: Cursor <cursoragent@cursor.com>
artem-shelkovnikov
approved these changes
Jul 8, 2026
Jan-Kazlouski-elastic
enabled auto-merge (squash)
July 8, 2026 08:36
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
2 participants
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.
Summary
Manual backport of #4119 to
8.19.The GitHub connector previously wrapped each fetch method in
try/except Exception: log warning, so a sync could "succeed" while syncing nothing (e.g. if no issues could be fetched at all). This ports the two-tier error handling, removes the broad exception handling, and hardens null-handling of GraphQL responses.The automated backport could not cherry-pick because
mainhas since split the GitHub connector into a package (app/connectors_service/connectors/sources/github/{client,datasource,utils}.py), whereas8.19still uses the single moduleconnectors/sources/github.py. The changes were therefore ported by hand into that single module.Changes
_fetch_repos/_fetch_pull_requests/_fetch_issues/_fetch_files/_fetch_files_by_path(so the sync fails when data genuinely can't be fetched), while per-document enrichment failures are WARN-logged and skipped via narrowgidgethub.GitHubExceptionguards.RateLimitingError._put_to_sleepnow only sleeps; its callers (get_github_item,graphql,_update_installation_access_token,_github_app_get) re-raiseRateLimitingError, which is not inskipped_exceptions, so the existing@retryablelayer retries it.except Exception: replaced withgidgethub.GitHubException(plus the connector's ownUnauthorizedException/ForbiddenExceptionin log-and-reraise spots likepingand_get_invalid_repos_for_personal_access_token).reviews,comments,labels,assignees,reviewRequests,tree, edge nodes) to the... or {}/... or []idiom so a single document withnullconnection fields can no longer abort the sync.RateLimitingErrorpropagation, null-connection-field regressions, and thedeepcopytest-isolation fixes.Test plan
make clean install autoformat lint test PYTHON=python3.11— lint clean;tests/sources/test_github.pypasses (121 passed), overall coverage 92.35%.pytest-randomlyordering flakiness intest_google_drive.py/test_google_cloud_storage.py(they pass in isolation and with a deterministic order); they are unrelated to these GitHub-only changes.Part of #2393, closes #2395 for 8.19.
Made with Cursor