fix(google-drive): report a failed download as a failure, not a skip

Merged
EV
1d ago
+104-2

Description

Stack B (quick wins in the Drive connector), B6. Plan section 1.2, Notes: "a failed download drops the file with no ConnectorFailure". This PR is independent: its base is main.

In _download_and_extract_sections_basic, two branches had the download inside a broad except Exception:

  • the last branch, which reads a file by its extension (for example Markdown, HTML, JSON, EPUB),
  • the image branch.

A failed download then gave a result with no sections. The caller logged "No content extracted" and returned no document and no failure. So:

  • the attempt had no error record for the file,
  • the run did not try the next user (the owner, then the admin) after a 403,
  • a rate limit that outlasted its retries lost the file, and no later run fetched it again until the file changed.

The download now runs before the try. A failed download raises into _convert_drive_item_to_document, which already turns an error into a ConnectorFailure. The user fallback in convert_drive_item_to_document then works for these types too.

Decisions

  • A file that cannot be parsed is still skipped. Only the download moved out of the try. A parse error is not a retriable condition.
  • A 403 for every user still skips the file, as for the other types: convert_drive_item_to_document logs it and returns no failure. This PR does not change that rule.

How Has This Been Tested?

New unit tests in test_drive_download_failure.py (5 tests). Four of them fail without the fix:

  • a 500 and a rate limit on the download give a ConnectorFailure with the document id,
  • a 403 for the listing user goes to the next user, and the file becomes a document,
  • a failed image download is a failure,
  • a file that cannot be parsed is still skipped (passes with and without the fix).

tests/unit/onyx/connectors/google_drive: 228 pass. ruff, ty, ods check-getattr, ods check-lazy-imports: pass.

Not run: the mock benchmark (no download fails in a clean run, so its numbers do not change), a live Workspace, the full pre-commit set and CI.

Docs

  • Customer-facing change: I updated documentation/ (text and screenshots).
  • No docs change needed (internal, tests, CI, or multi-tenant cloud only).

Credits

Dane (Danelegend) found this in #17542.

Additional Options

  • [Optional] Please cherry-pick this PR to the latest release version.

https://claude.ai/code/session_01SK7J8QCWWmwbihV7zceSKD

Generated by Claude Code

[!NOTE]

Report Google Drive download failures as ConnectorFailure instead of skipped files

  • Moves image and text downloads in the basic extraction function outside the exception handlers, so download errors (e.g. server or rate-limit errors) now propagate as a ConnectorFailure instead of producing an empty section result. Image-storage and text-parsing failures remain caught and logged as skips. Change is in doc_conversion.py.
  • Adds tests in test_drive_download_failure.py covering server/rate-limit download errors, a denied lister download followed by fallback to the next retriever email, failed image downloads, and unparseable files returning None.
  • Behavioral Change: Google Drive syncs that previously skipped files with download errors will now surface those files as connector failures.

Macroscope summarized e8ebcf6.

Summary by CodeRabbit

  • Bug Fixes
    • Google Drive download failures are now reported as connector failures instead of being silently treated as empty content.
    • Files that fail during content processing continue to be skipped, while access-denied listings can still fall back to the file owner and allow successful documents to be imported.