danielemoraschi commented on code in PR #8791:
URL:
https://github.com/apache/incubator-devlake/pull/8791#discussion_r3063141767
##########
backend/plugins/github/token/round_tripper.go:
##########
@@ -93,3 +95,36 @@ func (rt *RefreshRoundTripper) roundTripWithRetry(req
*http.Request, refreshAtte
return resp, nil
}
+
+// StaticRoundTripper is an HTTP transport that injects a fixed bearer token.
+// Unlike RefreshRoundTripper, it does NOT attempt refresh or retries.
+type StaticRoundTripper struct {
+ base http.RoundTripper
+ tokens []string
+ idx atomic.Uint64
+}
+
+func NewStaticRoundTripper(base http.RoundTripper, rawToken string)
*StaticRoundTripper {
+ if base == nil {
+ base = http.DefaultTransport
+ }
+ parts := strings.Split(rawToken, ",")
+ tokens := make([]string, 0, len(parts))
+ for _, t := range parts {
+ if t = strings.TrimSpace(t); t != "" {
+ tokens = append(tokens, t)
+ }
+ }
+ if len(tokens) == 0 {
+ tokens = []string{rawToken}
+ }
+ return &StaticRoundTripper{base: base, tokens: tokens}
+}
+
+func (rt *StaticRoundTripper) RoundTrip(req *http.Request) (*http.Response,
error) {
+ // always overrides headers put by SetupAuthentication, to make sure
the token is always injected
+ tok := rt.tokens[rt.idx.Add(1)%uint64(len(rt.tokens))]
Review Comment:
Uint64 starts at 0, `Add(1)` returns 1, so the first request uses
`tokens[1]`, not `tokens[0]`. For single-token lists this is harmless (1%1==0),
but multi-PAT connections skip the first token on first request. The old
SetupAuthentication started at index 0.
##########
backend/helpers/pluginhelper/api/graphql_async_client.go:
##########
@@ -126,7 +154,9 @@ func (apiClient *GraphqlAsyncClient)
updateRateRemaining(rateRemaining int, rese
case <-time.After(nextDuring):
newRateRemaining, newResetAt, err :=
apiClient.getRateRemaining(apiClient.ctx, apiClient.client, apiClient.logger)
if err != nil {
- panic(err)
+ apiClient.logger.Info("failed to update graphql
rate limit, will retry next cycle: %v", err)
+
apiClient.updateRateRemaining(apiClient.rateRemaining, nil)
Review Comment:
Potential deadlock on GitHub Enterprise? The Signal() at L139 only fires
when `rateRemaining > 0`, so all goroutines waiting in Query (L183: for
rateRemaining <= 0 { Wait() }) block forever. Since Github returns this error
every cycle rate limit never recovers.
Check if use `max(apiClient.rateRemaining, defaultRateLimitConst)` instead
of `apiClient.rateRemaining` works, to ensure the fallback is always positive
and Signal() fires.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
