[INC-300] baton-github: include the error in the returned message#98
[INC-300] baton-github: include the error in the returned message#98Bencheng21 merged 6 commits intomainfrom
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| }, | ||
| }) | ||
| if err != nil { | ||
| if isRatelimited(resp) { |
There was a problem hiding this comment.
I add isRatelimited to Every API calls, including List and Grants
| for { | ||
| orgs, resp, err := client.Organizations.List(ctx, "", &github.ListOptions{Page: page, PerPage: maxPageSize}) | ||
| if err != nil { | ||
| if isRatelimited(resp) { |
There was a problem hiding this comment.
that's Validate function, baton-sdk doesn't do the retry for Validate function, I don't know why we have to handle with this error.
There was a problem hiding this comment.
There was a problem hiding this comment.
and also the error message is not correct
can you return an error instead of nil?
There was a problem hiding this comment.
OK, that makes sense. TIL
| orgs, resp, err := o.client.Organizations.List(ctx, "", opts) | ||
| if err != nil { | ||
| if isRatelimited(resp) { | ||
| return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err) |
There was a problem hiding this comment.
Same here. Add isRatelimited to List()
| if err != nil { | ||
| if isRatelimited(res) { | ||
| return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err) | ||
| } |
There was a problem hiding this comment.
Bug: GitHub API Client Nil Response Handling
The isRatelimited function is called on GitHub API responses without a preceding nil check. GitHub API client methods can return a nil response object alongside an error (e.g., due to network issues or specific undocumented API behaviors like Users.GetByID), leading to nil pointer dereferences.
There was a problem hiding this comment.
isRatelimited has checked nil pointer.
2 participants
No description provided.