fix: google sheets timeout issue #6132
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR addresses timeout issues in the Google Sheets client by propagating a configurable timeout through service creation and OAuth token retrieval.
- Added a
timeoutparameter toclientOptions,generateService, andgenerateOAuthClient - Updated call sites in
NewProducerto pass the timeout value - Imported the
timepackage and usedcontext.WithTimeoutto enforce deadlines
Comments suppressed due to low confidence (3)
services/streammanager/googlesheets/googlesheetsmanager.go:147
- [nitpick] Update this comment to reflect the added
timeoutparameter, e.g., indicate thatgenerateServiceaccepts a timeout and applies it to the context.
// generateService produces a google-sheets client using the specified client options
services/streammanager/googlesheets/googlesheetsmanager.go:325
- [nitpick] Update this comment to mention the
timeoutargument ingenerateOAuthClient's signature so the contract is clear.
// generateOAuthClient produces an OAuth client based on a jwt Config
services/streammanager/googlesheets/googlesheetsmanager.go:295
- Add unit tests to verify that the
timeoutparameter is properly applied inclientOptions,generateOAuthClient, andgenerateService, ensuring deadlines are enforced when contexts expire.
func clientOptions(config *Config, timeout time.Duration) ([]option.ClientOption, error) {
| ctx := context.Background() | ||
| func generateOAuthClient(jwtconfig *jwt.Config, timeout time.Duration) (*http.Client, error) { | ||
| ctx, cancel := context.WithTimeout(context.Background(), timeout) | ||
| defer cancel() | ||
| var oauthconfig *oauth2.Config |
There was a problem hiding this comment.
The variable oauthconfig is declared but never initialized before use, leading to a nil pointer dereference when calling oauthconfig.Client. Initialize oauthconfig properly or use jwtconfig.Client directly.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
@manish339k Can you verify this too
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6132 +/- ##
==========================================
- Coverage 76.77% 76.70% -0.07%
==========================================
Files 521 521
Lines 70779 70781 +2
==========================================
- Hits 54340 54294 -46
- Misses 13548 13594 +46
- Partials 2891 2893 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
manish339k
force-pushed
the
fix.google-sheets-timeout-issue-2
branch
2 times, most recently
from
7d55dd6 to
10efc6d
Compare
|
This PR is considered to be stale. It has been open 20 days with no further activity thus it is going to be closed in 7 days. To avoid such a case please consider removing the stale label manually or add a comment to the PR. |
manish339k
force-pushed
the
fix.google-sheets-timeout-issue-2
branch
from
10efc6d to
be9f9a9
Compare
Description
The error "CDM GOOGLESHEETS Unable to create client for context deadline exceeded" was caused by several timeout-related issues in the Google Sheets client creation process:
Issues Fixed:
Missing Timeout Context in Service Creation: The generateService function was using context.Background() without any timeout when creating the Google Sheets service.
Missing Timeout Context in OAuth Token Retrieval: The generateOAuthClient function was also using context.Background() without timeout when retrieving OAuth tokens.
No Timeout Propagation: The timeout value from the custom destination manager wasn't being properly propagated through the Google Sheets client creation chain.
Changes Made:
Updated generateService to accept a timeout parameter and use context.WithTimeout()
Updated generateOAuthClient to accept a timeout parameter and use context.WithTimeout()
Updated clientOptions to accept and pass through the timeout parameter
Added time import to support timeout functionality
Updated call sites to pass the timeout value through the chain
Linear Ticket
https://linear.app/rudderstack/issue/INT-3950/google-sheet-investigate-1542-failed-events-on-facebook-feed-in
Security