-
Notifications
You must be signed in to change notification settings - Fork 3.2k
fix(google-vault): error handling improvement and more params #2735
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
waleedlatif1
commented
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
Adds specialized error handling for Google Vault credential expiration issues by creating a dedicated block handler that detects RAPT and invalid_grant errors, providing users with actionable reconnection instructions. Extends Google Vault functionality with date filtering (startTime, endTime) and search terms for both exports and holds, plus shared drive support for DRIVE corpus holds. Removes the unused timeZone parameter from export operations.
Confidence Score: 3/5
- Generally safe to merge with minor validation concerns
- The PR improves error handling and adds useful functionality, but has duplicate field IDs in the block configuration that could cause form rendering issues. The query object construction in create_matters_holds.ts should be verified to ensure empty query objects are handled correctly by the Google Vault API. The error handling logic itself is well-implemented with comprehensive pattern matching.
- apps/sim/blocks/blocks/google_vault.ts needs duplicate field ID resolution; apps/sim/tools/google_vault/create_matters_holds.ts needs API validation for empty query objects
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| apps/sim/executor/handlers/google-vault/google-vault-handler.ts | 4/5 | Adds specialized handler for Google Vault blocks with enhanced credential error detection and user-friendly error messages |
| apps/sim/tools/google_vault/create_matters_holds.ts | 3/5 | Adds query parameters for MAIL/GROUPS date filtering and DRIVE shared drive support; query object construction needs validation |
| apps/sim/blocks/blocks/google_vault.ts | 3/5 | Adds extensive UI configuration for date filtering and search terms with wand AI prompts; duplicate field IDs need attention |
Sequence Diagram
sequenceDiagram
participant Client
participant GoogleVaultHandler
participant executeTool
participant GoogleVaultAPI
Client->>GoogleVaultHandler: execute(block, inputs)
GoogleVaultHandler->>GoogleVaultHandler: Transform inputs & parse JSON
GoogleVaultHandler->>executeTool: Execute with credentials
executeTool->>GoogleVaultAPI: API Request
alt Success
GoogleVaultAPI-->>executeTool: Valid response
executeTool-->>GoogleVaultHandler: result.success = true
GoogleVaultHandler-->>Client: Return output
else Credential Error
GoogleVaultAPI-->>executeTool: invalid_grant/RAPT error
executeTool-->>GoogleVaultHandler: result.success = false
GoogleVaultHandler->>GoogleVaultHandler: isCredentialRefreshError()
GoogleVaultHandler->>GoogleVaultHandler: enhanceCredentialError()
GoogleVaultHandler-->>Client: User-friendly error with reconnection guidance
else Other Error
GoogleVaultAPI-->>executeTool: Other error
executeTool-->>GoogleVaultHandler: Throws error
GoogleVaultHandler->>GoogleVaultHandler: Check if credential error
GoogleVaultHandler-->>Client: Enhanced or standard error message
end
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate field ID startTime defined. This field already exists for create_matters_export operation (lines 162-184) and is redefined for create_matters_holds operation (lines 186-208). The UI form system may not handle duplicate field IDs correctly, potentially causing configuration conflicts or only one field being recognized.
Consider using unique field IDs like export_startTime and hold_startTime, or refactor to use a single field definition with more complex conditions if the behavior should be identical.
Prompt To Fix With AI
This is a comment left during a code review. Path: apps/sim/blocks/blocks/google_vault.ts Line: 162:184 Comment: Duplicate field ID `startTime` defined. This field already exists for create_matters_export operation (lines 162-184) and is redefined for create_matters_holds operation (lines 186-208). The UI form system may not handle duplicate field IDs correctly, potentially causing configuration conflicts or only one field being recognized. Consider using unique field IDs like `export_startTime` and `hold_startTime`, or refactor to use a single field definition with more complex conditions if the behavior should be identical. How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate field ID endTime defined. This field is already defined for create_matters_export (lines 186-208) and is redefined for create_matters_holds (lines 210-232). Similar to startTime, duplicate IDs may cause issues with the form rendering system.
Consider using unique field IDs to avoid potential conflicts.
Prompt To Fix With AI
This is a comment left during a code review. Path: apps/sim/blocks/blocks/google_vault.ts Line: 186:208 Comment: Duplicate field ID `endTime` defined. This field is already defined for create_matters_export (lines 186-208) and is redefined for create_matters_holds (lines 210-232). Similar to `startTime`, duplicate IDs may cause issues with the form rendering system. Consider using unique field IDs to avoid potential conflicts. How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate field ID terms defined. This field appears twice - once for exports (lines 234-256) and once for holds (lines 258-278). Like the date fields, duplicate field IDs may cause form rendering or data binding issues.
Consider using unique identifiers for each operation's search terms field.
Prompt To Fix With AI
This is a comment left during a code review. Path: apps/sim/blocks/blocks/google_vault.ts Line: 234:256 Comment: Duplicate field ID `terms` defined. This field appears twice - once for exports (lines 234-256) and once for holds (lines 258-278). Like the date fields, duplicate field IDs may cause form rendering or data binding issues. Consider using unique identifiers for each operation's search terms field. How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verify that the Google Vault API accepts empty query objects when no filtering parameters are provided. The current code creates { mailQuery: {} } or { groupsQuery: {} } structures when the corpus is MAIL or GROUPS but no terms or dates are specified. Check if the query field should be omitted entirely in this case to avoid potential API errors. Does the Google Vault API require the query field to be omitted when no filters are specified, or does it accept empty query objects?
Prompt To Fix With AI
This is a comment left during a code review. Path: apps/sim/tools/google_vault/create_matters_holds.ts Line: 101:119 Comment: Verify that the Google Vault API accepts empty query objects when no filtering parameters are provided. The current code creates `{ mailQuery: {} }` or `{ groupsQuery: {} }` structures when the corpus is MAIL or GROUPS but no terms or dates are specified. Check if the `query` field should be omitted entirely in this case to avoid potential API errors. Does the Google Vault API require the query field to be omitted when no filters are specified, or does it accept empty query objects? How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition params.includeSharedDrives only adds the driveQuery when truthy. If explicitly set to false, no query is added. Consider whether false should explicitly set includeSharedDriveFiles: false in the API request.
Prompt To Fix With AI
This is a comment left during a code review. Path: apps/sim/tools/google_vault/create_matters_holds.ts Line: 116:118 Comment: The condition `params.includeSharedDrives` only adds the driveQuery when truthy. If explicitly set to `false`, no query is added. Consider whether `false` should explicitly set `includeSharedDriveFiles: false` in the API request. ```suggestion } else if (params.corpus === 'DRIVE' && params.includeSharedDrives !== undefined) { body.query = { driveQuery: { includeSharedDriveFiles: params.includeSharedDrives } } } ``` How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented comprehensive error detection for Google Vault authentication issues, covering multiple error patterns including RAPT errors, invalid_grant with rapt, token refresh failures, and 401 access token errors
Prompt To Fix With AI
This is a comment left during a code review. Path: apps/sim/executor/handlers/google-vault/google-vault-handler.ts Line: 24:32 Comment: Implemented comprehensive error detection for Google Vault authentication issues, covering multiple error patterns including RAPT errors, invalid_grant with rapt, token refresh failures, and 401 access token errors How can I resolve this? If you propose a fix, please make it concise.
Summary
Added better error handling for vault specifically. Google Cloud by default has a limit for token expiration, and they throw invalid_grant after the token expires, and the only workaround to this is disconneting and reconnecting credential, or, as an admin, disabling reauth for Sim.
Also added additional params for dates
Type of Change
Testing
Tested manually
Checklist