Skip to content

Commit 551e7f7

Browse files
committed
fix(github-graphql): prevent panic in graphql rate limit polling goroutine
Replace panic in GraphqlAsyncClient rate-limit polling goroutine with graceful error handling. Previously, any error while fetching rate limit (e.g., transient network issues or 401 responses) would trigger a panic inside a background goroutine, crashing the entire DevLake process. Now, errors are logged and the client retries in the next cycle while retaining the last known rate limit. Design decisions: - Avoid panic in background goroutines: rate-limit polling is non-critical and should not bring down the entire pipeline. - Use last known rateRemaining on runtime failures instead of resetting or blocking, ensuring continued progress with eventual consistency. - Retry via existing polling mechanism instead of immediate retry to prevent tight retry loops and unnecessary API pressure. - Introduce a default fallback (5000) only for initial rate-limit fetch failures, since no prior state exists at startup. - Separate handling of initial vs runtime failures: - Initial failure → fallback to default (5000) - Runtime failure → retain previous value Fixes #8788 (bug 1)
1 parent b68c102 commit 551e7f7

1 file changed

Lines changed: 9 additions & 3 deletions

File tree

backend/helpers/pluginhelper/api/graphql_async_client.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ type GraphqlAsyncClient struct {
4747
getRateCost func(q interface{}) int
4848
}
4949

50+
const defaultRateLimit = 5000
51+
5052
// CreateAsyncGraphqlClient creates a new GraphqlAsyncClient
5153
func CreateAsyncGraphqlClient(
5254
taskCtx plugin.TaskContext,
@@ -68,9 +70,11 @@ func CreateAsyncGraphqlClient(
6870
if getRateRemaining != nil {
6971
rateRemaining, resetAt, err := getRateRemaining(taskCtx.GetContext(), graphqlClient, logger)
7072
if err != nil {
71-
panic(err)
73+
graphqlAsyncClient.logger.Warn(err, "failed to fetch initial graphql rate limit, fallback to default")
74+
graphqlAsyncClient.updateRateRemaining(defaultRateLimit, nil)
75+
} else {
76+
graphqlAsyncClient.updateRateRemaining(rateRemaining, resetAt)
7277
}
73-
graphqlAsyncClient.updateRateRemaining(rateRemaining, resetAt)
7478
}
7579

7680
// load retry/timeout from configuration
@@ -126,7 +130,9 @@ func (apiClient *GraphqlAsyncClient) updateRateRemaining(rateRemaining int, rese
126130
case <-time.After(nextDuring):
127131
newRateRemaining, newResetAt, err := apiClient.getRateRemaining(apiClient.ctx, apiClient.client, apiClient.logger)
128132
if err != nil {
129-
panic(err)
133+
apiClient.logger.Warn(err, "failed to update graphql rate limit, will retry next cycle")
134+
apiClient.updateRateRemaining(apiClient.rateRemaining, nil)
135+
return
130136
}
131137
apiClient.updateRateRemaining(newRateRemaining, newResetAt)
132138
}

0 commit comments

Comments
 (0)