fix: increase Gemini API timeout from 30s to configurable 120s for large video#19
Open
onurege3467 wants to merge 1 commit into
Open
fix: increase Gemini API timeout from 30s to configurable 120s for large video#19onurege3467 wants to merge 1 commit into
onurege3467 wants to merge 1 commit into
Conversation
…rge video support Closes clencyc#16 The Google GenAI Python SDK has a default HTTP timeout of about 30 seconds, which causes timeout errors when uploading/analyzing videos larger than 100MB. Changes: - Add GEMINI_API_TIMEOUT env var (default 120s) to ai_client.py - Pass it as http_options to genai.Client() in both API key and Vertex AI paths (timeout is in milliseconds for the SDK) - Document the new setting in .env.example - Improve timeout error messaging in VideoIngestionPanel.tsx
|
Someone is attempting to deploy a commit to the ClaraClency Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe PR implements configurable Gemini API timeout support to handle large video uploads without timing out after the SDK default of 30 seconds. The backend reads ChangesGemini API Timeout Configuration
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
LiveEditFronten/components/VideoIngestionPanel.tsx (1)
62-63: 💤 Low valueConsider case-insensitive timeout detection.
The current timeout detection checks three string variants (
'timeout','Timeout','timed out'). A case-insensitive regex would be more robust and concise.♻️ Optional refactor using regex
- if (msg.includes('timeout') || msg.includes('Timeout') || msg.includes('timed out')) { + if (/timeout|timed out/i.test(msg)) { setError(`The video analysis timed out. Try a smaller file or increase GEMINI_API_TIMEOUT in the backend .env file.`); } else {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@LiveEditFronten/components/VideoIngestionPanel.tsx` around lines 62 - 63, The timeout detection in VideoIngestionPanel.tsx currently checks msg with multiple case variants; update the check that sets setError(...) to use a single case-insensitive test (e.g., a regex with the i flag) against the msg variable so any casing or phrasing like "timeout" / "timed out" is matched robustly and concisely; locate the block where msg is inspected and replace the multiple msg.includes(...) checks with one case-insensitive match before calling setError.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@LiveEditBackend/ai_client.py`:
- Line 28: The module-level conversion of GEMINI_API_TIMEOUT can raise
ValueError for non-integer env values; update the GEMINI_API_TIMEOUT assignment
in ai_client.py to validate the raw env string (os.getenv("GEMINI_API_TIMEOUT"))
with a try/except around int(...) so you catch ValueError, log a warning (use
the module logger or Python logging) that the value was invalid, and fall back
to the default integer 120; ensure the symbol GEMINI_API_TIMEOUT still ends up
as an int usable by other functions.
In `@LiveEditFronten/components/VideoIngestionPanel.tsx`:
- Around line 33-48: The blob URL created with URL.createObjectURL(file) is only
revoked in the onloadedmetadata success handler, causing a leak if onerror fires
or an exception occurs; update the logic around the temporary video element (the
URL.createObjectURL, video element, video.onloadedmetadata and video.onerror
handlers and the surrounding try block) to ensure URL.revokeObjectURL(url) is
called in all paths (onloadedmetadata, onerror, and in a finally/fallback for
the try) so the created object URL is always cleaned up.
---
Nitpick comments:
In `@LiveEditFronten/components/VideoIngestionPanel.tsx`:
- Around line 62-63: The timeout detection in VideoIngestionPanel.tsx currently
checks msg with multiple case variants; update the check that sets setError(...)
to use a single case-insensitive test (e.g., a regex with the i flag) against
the msg variable so any casing or phrasing like "timeout" / "timed out" is
matched robustly and concisely; locate the block where msg is inspected and
replace the multiple msg.includes(...) checks with one case-insensitive match
before calling setError.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: da207605-faa4-49f6-846a-6ab1d08bfc7c
📒 Files selected for processing (3)
LiveEditBackend/.env.exampleLiveEditBackend/ai_client.pyLiveEditFronten/components/VideoIngestionPanel.tsx
| API_KEY = (os.getenv("GEMINI_API_KEY") or "").strip() | ||
| TEXT_MODEL_NAME = os.getenv("GEMINI_TEXT_MODEL", "gemini-2.0-flash").strip() or "gemini-2.0-flash" | ||
| VIDEO_MODEL_NAME = os.getenv("GEMINI_VIDEO_MODEL", "gemini-2.0-flash").strip() or "gemini-2.0-flash" | ||
| GEMINI_API_TIMEOUT = int(os.getenv("GEMINI_API_TIMEOUT", "120")) |
There was a problem hiding this comment.
Add error handling for invalid GEMINI_API_TIMEOUT values.
If GEMINI_API_TIMEOUT is set to a non-integer value (e.g., GEMINI_API_TIMEOUT=abc), the code will crash with a ValueError at module load time, preventing the entire backend from starting.
🛡️ Proposed fix with validation and error handling
-GEMINI_API_TIMEOUT = int(os.getenv("GEMINI_API_TIMEOUT", "120"))
+def _parse_timeout(default: int = 120) -> int:
+ """Parse GEMINI_API_TIMEOUT from env, with validation."""
+ raw = os.getenv("GEMINI_API_TIMEOUT", str(default))
+ try:
+ timeout = int(raw)
+ if timeout <= 0:
+ raise ValueError(f"GEMINI_API_TIMEOUT must be positive, got {timeout}")
+ return timeout
+ except ValueError as e:
+ raise ValueError(
+ f"Invalid GEMINI_API_TIMEOUT='{raw}': must be a positive integer. {e}"
+ )
+
+GEMINI_API_TIMEOUT = _parse_timeout()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| GEMINI_API_TIMEOUT = int(os.getenv("GEMINI_API_TIMEOUT", "120")) | |
| def _parse_timeout(default: int = 120) -> int: | |
| """Parse GEMINI_API_TIMEOUT from env, with validation.""" | |
| raw = os.getenv("GEMINI_API_TIMEOUT", str(default)) | |
| try: | |
| timeout = int(raw) | |
| if timeout <= 0: | |
| raise ValueError(f"GEMINI_API_TIMEOUT must be positive, got {timeout}") | |
| return timeout | |
| except ValueError as e: | |
| raise ValueError( | |
| f"Invalid GEMINI_API_TIMEOUT='{raw}': must be a positive integer. {e}" | |
| ) | |
| GEMINI_API_TIMEOUT = _parse_timeout() |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@LiveEditBackend/ai_client.py` at line 28, The module-level conversion of
GEMINI_API_TIMEOUT can raise ValueError for non-integer env values; update the
GEMINI_API_TIMEOUT assignment in ai_client.py to validate the raw env string
(os.getenv("GEMINI_API_TIMEOUT")) with a try/except around int(...) so you catch
ValueError, log a warning (use the module logger or Python logging) that the
value was invalid, and fall back to the default integer 120; ensure the symbol
GEMINI_API_TIMEOUT still ends up as an int usable by other functions.
Comment on lines
+33
to
+48
| // Try to get video duration from a temporary element | ||
| let duration = 0; | ||
| try { | ||
| // Try to get video duration from a temporary element | ||
| let duration = 0; | ||
| try { | ||
| const url = URL.createObjectURL(file); | ||
| const video = document.createElement('video'); | ||
| video.preload = 'metadata'; | ||
| await new Promise<void>((resolve) => { | ||
| video.onloadedmetadata = () => { | ||
| duration = video.duration; | ||
| URL.revokeObjectURL(url); | ||
| resolve(); | ||
| }; | ||
| video.onerror = () => resolve(); | ||
| video.src = url; | ||
| }); | ||
| } catch { /* ignore */ } | ||
| const url = URL.createObjectURL(file); | ||
| const video = document.createElement('video'); | ||
| video.preload = 'metadata'; | ||
| await new Promise<void>((resolve) => { | ||
| video.onloadedmetadata = () => { | ||
| duration = video.duration; | ||
| URL.revokeObjectURL(url); | ||
| resolve(); | ||
| }; | ||
| video.onerror = () => resolve(); | ||
| video.src = url; | ||
| }); | ||
| } catch { /* ignore */ } |
There was a problem hiding this comment.
Fix resource leak: revoke URL in all code paths.
The URL.revokeObjectURL(url) is only called in the success path (line 42). If the video fails to load (onerror fires on line 45) or the outer try block throws, the blob URL is never revoked, causing a memory leak that accumulates with repeated uploads.
🔧 Proposed fix to ensure cleanup in all paths
// Try to get video duration from a temporary element
let duration = 0;
try {
const url = URL.createObjectURL(file);
- const video = document.createElement('video');
- video.preload = 'metadata';
- await new Promise<void>((resolve) => {
- video.onloadedmetadata = () => {
- duration = video.duration;
- URL.revokeObjectURL(url);
- resolve();
- };
- video.onerror = () => resolve();
- video.src = url;
- });
+ try {
+ const video = document.createElement('video');
+ video.preload = 'metadata';
+ await new Promise<void>((resolve) => {
+ video.onloadedmetadata = () => {
+ duration = video.duration;
+ resolve();
+ };
+ video.onerror = () => resolve();
+ video.src = url;
+ });
+ } finally {
+ URL.revokeObjectURL(url);
+ }
} catch { /* ignore */ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Try to get video duration from a temporary element | |
| let duration = 0; | |
| try { | |
| // Try to get video duration from a temporary element | |
| let duration = 0; | |
| try { | |
| const url = URL.createObjectURL(file); | |
| const video = document.createElement('video'); | |
| video.preload = 'metadata'; | |
| await new Promise<void>((resolve) => { | |
| video.onloadedmetadata = () => { | |
| duration = video.duration; | |
| URL.revokeObjectURL(url); | |
| resolve(); | |
| }; | |
| video.onerror = () => resolve(); | |
| video.src = url; | |
| }); | |
| } catch { /* ignore */ } | |
| const url = URL.createObjectURL(file); | |
| const video = document.createElement('video'); | |
| video.preload = 'metadata'; | |
| await new Promise<void>((resolve) => { | |
| video.onloadedmetadata = () => { | |
| duration = video.duration; | |
| URL.revokeObjectURL(url); | |
| resolve(); | |
| }; | |
| video.onerror = () => resolve(); | |
| video.src = url; | |
| }); | |
| } catch { /* ignore */ } | |
| // Try to get video duration from a temporary element | |
| let duration = 0; | |
| try { | |
| const url = URL.createObjectURL(file); | |
| try { | |
| const video = document.createElement('video'); | |
| video.preload = 'metadata'; | |
| await new Promise<void>((resolve) => { | |
| video.onloadedmetadata = () => { | |
| duration = video.duration; | |
| resolve(); | |
| }; | |
| video.onerror = () => resolve(); | |
| video.src = url; | |
| }); | |
| } finally { | |
| URL.revokeObjectURL(url); | |
| } | |
| } catch { /* ignore */ } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@LiveEditFronten/components/VideoIngestionPanel.tsx` around lines 33 - 48, The
blob URL created with URL.createObjectURL(file) is only revoked in the
onloadedmetadata success handler, causing a leak if onerror fires or an
exception occurs; update the logic around the temporary video element (the
URL.createObjectURL, video element, video.onloadedmetadata and video.onerror
handlers and the surrounding try block) to ensure URL.revokeObjectURL(url) is
called in all paths (onloadedmetadata, onerror, and in a finally/fallback for
the try) so the created object URL is always cleaned up.
❌ Deploy Preview for livedit failed. Why did it fail? →
|
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
1 participant
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.
Closes #16
The Google GenAI Python SDK has a default HTTP timeout of about 30
seconds, which causes requests.exceptions.Timeout errors when
uploading and analyzing videos larger than 100MB.
Changes:
Vertex AI paths (SDK expects timeout in milliseconds)
know they can increase GEMINI_API_TIMEOUT if they hit this error
The timeout is configurable per-environment: set GEMINI_API_TIMEOUT=300
in LiveEditBackend/.env for a 5-minute timeout on very large files.
Summary by CodeRabbit
New Features
Bug Fixes