extra logging for uncommon permissions cases #4532
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR enhances logging and error handling for Google Drive permission-related issues, particularly focusing on the 403 error scenario where users can see but not download files.
- Added debug logging in
backend/onyx/connectors/google_drive/doc_conversion.pyto capture 403 errors with retriever_email context - Refactored service creation in
doc_conversion.pyto use lazy evaluation for better resource management - Enhanced error messages in
backend/onyx/connectors/google_drive/connector.pyto include retrieval stage, user email, and parent drive/folder ID - Removed unused imports and functions to improve code cleanliness
- Standardized web view link handling with new WEB_VIEW_LINK_KEY constant
2 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile
| Main entry point for converting a Google Drive file => Document object. | ||
| """ | ||
| doc_id = "" | ||
| doc_id = file.get(WEB_VIEW_LINK_KEY, "") |
There was a problem hiding this comment.
logic: doc_id is overwritten on line 376, making this initial assignment redundant
| doc_id = file.get(WEB_VIEW_LINK_KEY, "") |
There was a problem hiding this comment.
Unfortunately not (the assignment on 376 is inside the try-except, so this assignment is used in cases where we error out before then). Good try Greptile 🥲
| if isinstance(e, HttpError) and e.status_code == 403: | ||
| logger.debug( | ||
| f"Uncommon permissions error while downloading file. User " | ||
| f"{retriever_email} was able to see file {file_name} " | ||
| "but cannot download it." | ||
| ) | ||
| logger.debug(error_str) |
There was a problem hiding this comment.
style: using debug level for permission errors might cause important troubleshooting info to be missed - consider using warning level instead
| file_name = file.get("name") | ||
| error_str = f"Error converting file '{file_name}' to Document: {e}" | ||
| if isinstance(e, HttpError) and e.status_code == 403: | ||
| logger.debug( |
| Main entry point for converting a Google Drive file => Document object. | ||
| """ | ||
| doc_id = "" | ||
| doc_id = file.get(WEB_VIEW_LINK_KEY, "") |
* extra logging for uncommon permissions cases * address CW comments
* extra logging for uncommon permissions cases * address CW comments
2 participants
Description
Addresses https://linear.app/danswer/issue/DAN-1817/logging-for-cases-where-user-can-see-but-not-download-from-drive
Extra logs for drive plus a small refactor. We've seen some cases where a user can list a file (i.e. they see it exists) but get a 403 when trying to download it. This logging will bring extra clarity about which users this is happening for and help us debug for a longer-term fix.
How Has This Been Tested?
n/a, will rely on connector tests for this since the change is relatively minor
Backporting (check the box to trigger backport action)
Note: You have to check that the action passes, otherwise resolve the conflicts manually and tag the patches.