Repository navigation
fix(files): prevent infinite loop on failed video processing and guard against nil response in Go - #121
Conversation
…d against nil response in Go - In Go, Python, and JavaScript files samples, poll loops checking for ACTIVE state previously hung indefinitely if a video file entered the FAILED state. Add explicit checks for FAILED state to error out immediately. - In go/test_utils.go, guard printResponse against nil *genai.GenerateContentResponse to prevent nil pointer dereference panics. - Add unit tests for printResponse nil and empty handling in go/test_utils_test.go. Signed-off-by: reginaldalfret <reginaldalfret@gmail.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces error handling during video file processing across Go, JavaScript, and Python implementations by checking for a failed state during polling. It also adds nil-safety and unit tests for printResponse in Go. Feedback recommends returning an error instead of calling log.Fatalf in Go, and adjusting the fallback logic for myfile.error in Python to handle cases where the attribute exists but is None.
| if myfile.State == genai.FileStateFailed { | ||
| log.Fatalf("video file processing failed: %v", myfile.State) | ||
| } |
There was a problem hiding this comment.
Instead of terminating the entire application with log.Fatalf, it is more idiomatic in Go to return the error to the caller, especially since the function signature FilesCreateVideo() (*genai.GenerateContentResponse, error) already supports returning an error. This allows callers (and unit tests) to handle the failure gracefully.
| if myfile.State == genai.FileStateFailed { | |
| log.Fatalf("video file processing failed: %v", myfile.State) | |
| } | |
| if myfile.State == genai.FileStateFailed { | |
| return nil, fmt.Errorf("video file processing failed: %v", myfile.State) | |
| } |
| if myfile.state and myfile.state.name == "FAILED": | ||
| raise ValueError(f"Video file processing failed: {getattr(myfile, 'error', 'state is FAILED')}") |
There was a problem hiding this comment.
In Python, getattr(myfile, 'error', 'state is FAILED') will return None if the error attribute exists on myfile but is set to None (which is common for Pydantic models or classes with predefined fields). To ensure the fallback string is used when error is None or falsy, use getattr(myfile, 'error', None) or 'state is FAILED'.
| if myfile.state and myfile.state.name == "FAILED": | |
| raise ValueError(f"Video file processing failed: {getattr(myfile, 'error', 'state is FAILED')}") | |
| if myfile.state and myfile.state.name == "FAILED": | |
| raise ValueError(f"Video file processing failed: {getattr(myfile, 'error', None) or 'state is FAILED'}") |
…n video failure Signed-off-by: reginaldalfret <reginaldalfret@gmail.com>
|
LG, thanks! |
Summary
This PR addresses video polling termination and a nil pointer dereference:
In \go/files.go, \python/files.py, and \javascript/files.js, the polling loops checking for video processing state only verified that the state was not yet \ACTIVE. If video processing encountered a failure on the backend (reaching state \FAILED), the loops never terminated and polled indefinitely until CI or application timeout. Explicit checks for \FAILED\ state were added across Go, Python, and JavaScript to fail fast with informative errors.
In \go/test_utils.go, \printResponse()\ did not guard against a
il\ *genai.GenerateContentResponse, causing a nil pointer dereference panic if called on failed/empty responses. Added an early return check and unit tests in \go/test_utils_test.go.
Validation