Skip to content

Commit 3d40b35

Browse files
Kshitij-Katiyarwiggin77
Kshitij-Katiyar
and
wiggin77
authored
[MM-809]: Fixed the issue of getting errors when using github api with revoked/invalid token (#832)
* [MM-809]: fix the issue of getting errors when using github api with revoked/invalid token * [MM-809]: Added proper error log in using github client * [MM-809]: Fixed lint * Update server/plugin/plugin.go * Update server/plugin/plugin.go * [MM-809]: review fixes --------- Co-authored-by: Doug Lauder <[email protected]>
1 parent 48fc3be commit 3d40b35

File tree

6 files changed

+363
-113
lines changed

6 files changed

+363
-113
lines changed

‎server/plugin/api.go‎

Lines changed: 147 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -666,10 +666,18 @@ func (p *Plugin) getMentions(c *UserContext, w http.ResponseWriter, r *http.Requ
666666
orgList := p.configuration.getOrganizations()
667667
query := getMentionSearchQuery(username, orgList)
668668

669-
result, _, err := githubClient.Search.Issues(c.Ctx, query, &github.SearchOptions{})
670-
if err != nil {
669+
var result *github.IssuesSearchResult
670+
var err error
671+
cErr := p.useGitHubClient(c.GHInfo, func(info *GitHubUserInfo, token *oauth2.Token) error {
672+
result, _, err = githubClient.Search.Issues(c.Ctx, query, &github.SearchOptions{})
673+
if err != nil {
674+
return err
675+
}
676+
return nil
677+
})
678+
if cErr != nil {
671679
p.writeAPIError(w, &APIErrorResponse{Message: "failed to search for issues", StatusCode: http.StatusInternalServerError})
672-
c.Log.WithError(err).With(logger.LogContext{"query": query}).Warnf("Failed to search for issues")
680+
c.Log.WithError(cErr).With(logger.LogContext{"query": query}).Warnf("Failed to search for issues")
673681
return
674682
}
675683

@@ -678,9 +686,17 @@ func (p *Plugin) getMentions(c *UserContext, w http.ResponseWriter, r *http.Requ
678686

679687
func (p *Plugin) getUnreadsData(c *UserContext) []*FilteredNotification {
680688
githubClient := p.githubConnectUser(c.Context.Ctx, c.GHInfo)
681-
notifications, _, err := githubClient.Activity.ListNotifications(c.Ctx, &github.NotificationListOptions{})
682-
if err != nil {
683-
c.Log.WithError(err).Warnf("Failed to list notifications")
689+
var notifications []*github.Notification
690+
var err error
691+
cErr := p.useGitHubClient(c.GHInfo, func(info *GitHubUserInfo, token *oauth2.Token) error {
692+
notifications, _, err = githubClient.Activity.ListNotifications(c.Ctx, &github.NotificationListOptions{})
693+
if err != nil {
694+
return err
695+
}
696+
return nil
697+
})
698+
if cErr != nil {
699+
c.Log.WithError(cErr).Warnf("Failed to list notifications")
684700
return nil
685701
}
686702

@@ -819,9 +835,17 @@ func (p *Plugin) searchIssues(c *UserContext, w http.ResponseWriter, r *http.Req
819835
allIssues := []*github.Issue{}
820836
for _, org := range orgsList {
821837
query := getIssuesSearchQuery(org, searchTerm)
822-
result, _, err := githubClient.Search.Issues(c.Ctx, query, &github.SearchOptions{})
823-
if err != nil {
824-
c.Log.WithError(err).With(logger.LogContext{"query": query}).Warnf("Failed to search for issues")
838+
var result *github.IssuesSearchResult
839+
var err error
840+
cErr := p.useGitHubClient(c.GHInfo, func(info *GitHubUserInfo, token *oauth2.Token) error {
841+
result, _, err = githubClient.Search.Issues(c.Ctx, query, &github.SearchOptions{})
842+
if err != nil {
843+
return err
844+
}
845+
return nil
846+
})
847+
if cErr != nil {
848+
c.Log.WithError(cErr).With(logger.LogContext{"query": query}).Warnf("Failed to search for issues")
825849
p.writeJSON(w, make([]*github.Issue, 0))
826850
return
827851
}
@@ -937,12 +961,24 @@ func (p *Plugin) createIssueComment(c *UserContext, w http.ResponseWriter, r *ht
937961
Body: &req.Comment,
938962
}
939963

940-
result, rawResponse, err := githubClient.Issues.CreateComment(c.Ctx, req.Owner, req.Repo, req.Number, comment)
941-
if err != nil {
964+
var result *github.IssueComment
965+
var rawResponse *github.Response
966+
if cErr := p.useGitHubClient(c.GHInfo, func(info *GitHubUserInfo, token *oauth2.Token) error {
967+
result, rawResponse, err = githubClient.Issues.CreateComment(c.Ctx, req.Owner, req.Repo, req.Number, comment)
968+
if err != nil {
969+
return err
970+
}
971+
return nil
972+
}); cErr != nil {
942973
statusCode := 500
943974
if rawResponse != nil {
944975
statusCode = rawResponse.StatusCode
945976
}
977+
c.Log.WithError(err).With(logger.LogContext{
978+
"owner": req.Owner,
979+
"repo": req.Repo,
980+
"number": req.Number,
981+
}).Errorf("failed to create an issue comment")
946982
p.writeAPIError(w, &APIErrorResponse{ID: "", Message: "failed to create an issue comment: " + getFailReason(statusCode, req.Repo, currentUsername), StatusCode: statusCode})
947983
return
948984
}
@@ -1008,9 +1044,8 @@ func (p *Plugin) getSidebarContent(c *UserContext, w http.ResponseWriter, r *htt
10081044

10091045
func (p *Plugin) postToDo(c *UserContext, w http.ResponseWriter, r *http.Request) {
10101046
githubClient := p.githubConnectUser(c.Context.Ctx, c.GHInfo)
1011-
username := c.GHInfo.GitHubUsername
10121047

1013-
text, err := p.GetToDo(c.Ctx, username, githubClient)
1048+
text, err := p.GetToDo(c.Ctx, c.GHInfo, githubClient)
10141049
if err != nil {
10151050
c.Log.WithError(err).Warnf("Failed to get Todos")
10161051
p.writeAPIError(w, &APIErrorResponse{ID: "", Message: "Encountered an error getting the to do items.", StatusCode: http.StatusUnauthorized})
@@ -1064,12 +1099,18 @@ func (p *Plugin) getIssueByNumber(c *UserContext, w http.ResponseWriter, r *http
10641099

10651100
githubClient := p.githubConnectUser(c.Context.Ctx, c.GHInfo)
10661101

1067-
result, _, err := githubClient.Issues.Get(c.Ctx, owner, repo, numberInt)
1068-
if err != nil {
1102+
var result *github.Issue
1103+
if cErr := p.useGitHubClient(c.GHInfo, func(info *GitHubUserInfo, token *oauth2.Token) error {
1104+
result, _, err = githubClient.Issues.Get(c.Ctx, owner, repo, numberInt)
1105+
if err != nil {
1106+
return err
1107+
}
1108+
return nil
1109+
}); cErr != nil {
10691110
// If the issue is not found, it's probably behind a private repo.
1070-
// Return an empty repose in this case.
1111+
// Return an empty response in this case.
10711112
var gerr *github.ErrorResponse
1072-
if errors.As(err, &gerr) && gerr.Response.StatusCode == http.StatusNotFound {
1113+
if errors.As(cErr, &gerr) && gerr.Response.StatusCode == http.StatusNotFound {
10731114
c.Log.WithError(err).With(logger.LogContext{
10741115
"owner": owner,
10751116
"repo": repo,
@@ -1079,7 +1120,7 @@ func (p *Plugin) getIssueByNumber(c *UserContext, w http.ResponseWriter, r *http
10791120
return
10801121
}
10811122

1082-
c.Log.WithError(err).With(logger.LogContext{
1123+
c.Log.WithError(cErr).With(logger.LogContext{
10831124
"owner": owner,
10841125
"repo": repo,
10851126
"number": numberInt,
@@ -1105,13 +1146,18 @@ func (p *Plugin) getPrByNumber(c *UserContext, w http.ResponseWriter, r *http.Re
11051146
}
11061147

11071148
githubClient := p.githubConnectUser(c.Context.Ctx, c.GHInfo)
1108-
1109-
result, _, err := githubClient.PullRequests.Get(c.Ctx, owner, repo, numberInt)
1110-
if err != nil {
1149+
var result *github.PullRequest
1150+
if cErr := p.useGitHubClient(c.GHInfo, func(userInfo *GitHubUserInfo, token *oauth2.Token) error {
1151+
result, _, err = githubClient.PullRequests.Get(c.Ctx, owner, repo, numberInt)
1152+
if err != nil {
1153+
return err
1154+
}
1155+
return nil
1156+
}); cErr != nil {
11111157
// If the pull request is not found, it's probably behind a private repo.
11121158
// Return an empty repose in this case.
11131159
var gerr *github.ErrorResponse
1114-
if errors.As(err, &gerr) && gerr.Response.StatusCode == http.StatusNotFound {
1160+
if errors.As(cErr, &gerr) && gerr.Response.StatusCode == http.StatusNotFound {
11151161
c.Log.With(logger.LogContext{
11161162
"owner": owner,
11171163
"repo": repo,
@@ -1122,7 +1168,7 @@ func (p *Plugin) getPrByNumber(c *UserContext, w http.ResponseWriter, r *http.Re
11221168
return
11231169
}
11241170

1125-
c.Log.WithError(err).With(logger.LogContext{
1171+
c.Log.WithError(cErr).With(logger.LogContext{
11261172
"owner": owner,
11271173
"repo": repo,
11281174
"number": numberInt,
@@ -1148,9 +1194,19 @@ func (p *Plugin) getLabels(c *UserContext, w http.ResponseWriter, r *http.Reques
11481194
opt := github.ListOptions{PerPage: 50}
11491195

11501196
for {
1151-
labels, resp, err := githubClient.Issues.ListLabels(c.Ctx, owner, repo, &opt)
1152-
if err != nil {
1153-
c.Log.WithError(err).Warnf("Failed to list labels")
1197+
var labels []*github.Label
1198+
var resp *github.Response
1199+
if cErr := p.useGitHubClient(c.GHInfo, func(info *GitHubUserInfo, token *oauth2.Token) error {
1200+
labels, resp, err = githubClient.Issues.ListLabels(c.Ctx, owner, repo, &opt)
1201+
if err != nil {
1202+
return err
1203+
}
1204+
return nil
1205+
}); cErr != nil {
1206+
c.Log.WithError(cErr).With(logger.LogContext{
1207+
"owner": owner,
1208+
"repo": repo,
1209+
}).Warnf("Failed to list labels")
11541210
p.writeAPIError(w, &APIErrorResponse{Message: "Failed to fetch labels", StatusCode: http.StatusInternalServerError})
11551211
return
11561212
}
@@ -1176,9 +1232,19 @@ func (p *Plugin) getAssignees(c *UserContext, w http.ResponseWriter, r *http.Req
11761232
opt := github.ListOptions{PerPage: 50}
11771233

11781234
for {
1179-
assignees, resp, err := githubClient.Issues.ListAssignees(c.Ctx, owner, repo, &opt)
1180-
if err != nil {
1181-
c.Log.WithError(err).Warnf("Failed to list assignees")
1235+
var assignees []*github.User
1236+
var resp *github.Response
1237+
if cErr := p.useGitHubClient(c.GHInfo, func(info *GitHubUserInfo, token *oauth2.Token) error {
1238+
assignees, resp, err = githubClient.Issues.ListAssignees(c.Ctx, owner, repo, &opt)
1239+
if err != nil {
1240+
return err
1241+
}
1242+
return nil
1243+
}); cErr != nil {
1244+
c.Log.WithError(cErr).With(logger.LogContext{
1245+
"owner": owner,
1246+
"repo": repo,
1247+
}).Warnf("Failed to list assignees")
11821248
p.writeAPIError(w, &APIErrorResponse{Message: "Failed to fetch assignees", StatusCode: http.StatusInternalServerError})
11831249
return
11841250
}
@@ -1204,9 +1270,19 @@ func (p *Plugin) getMilestones(c *UserContext, w http.ResponseWriter, r *http.Re
12041270
opt := github.ListOptions{PerPage: 50}
12051271

12061272
for {
1207-
milestones, resp, err := githubClient.Issues.ListMilestones(c.Ctx, owner, repo, &github.MilestoneListOptions{ListOptions: opt})
1208-
if err != nil {
1209-
c.Log.WithError(err).Warnf("Failed to list milestones")
1273+
var milestones []*github.Milestone
1274+
var resp *github.Response
1275+
if cErr := p.useGitHubClient(c.GHInfo, func(info *GitHubUserInfo, token *oauth2.Token) error {
1276+
milestones, resp, err = githubClient.Issues.ListMilestones(c.Ctx, owner, repo, &github.MilestoneListOptions{ListOptions: opt})
1277+
if err != nil {
1278+
return err
1279+
}
1280+
return nil
1281+
}); cErr != nil {
1282+
c.Log.WithError(cErr).With(logger.LogContext{
1283+
"owner": owner,
1284+
"repo": repo,
1285+
}).Warnf("Failed to list milestones")
12101286
p.writeAPIError(w, &APIErrorResponse{Message: "Failed to fetch milestones", StatusCode: http.StatusInternalServerError})
12111287
return
12121288
}
@@ -1220,12 +1296,21 @@ func (p *Plugin) getMilestones(c *UserContext, w http.ResponseWriter, r *http.Re
12201296
p.writeJSON(w, allMilestones)
12211297
}
12221298

1223-
func getRepositoryList(c context.Context, userName string, githubClient *github.Client, opt github.ListOptions) ([]*github.Repository, error) {
1299+
func (p *Plugin) getRepositoryList(c context.Context, ghInfo *GitHubUserInfo, userName string, githubClient *github.Client, opt github.ListOptions) ([]*github.Repository, error) {
12241300
var allRepos []*github.Repository
12251301
for {
1226-
repos, resp, err := githubClient.Repositories.List(c, userName, &github.RepositoryListOptions{ListOptions: opt})
1227-
if err != nil {
1228-
return nil, err
1302+
var repos []*github.Repository
1303+
var resp *github.Response
1304+
var err error
1305+
cErr := p.useGitHubClient(ghInfo, func(info *GitHubUserInfo, token *oauth2.Token) error {
1306+
repos, resp, err = githubClient.Repositories.List(c, userName, &github.RepositoryListOptions{ListOptions: opt})
1307+
if err != nil {
1308+
return err
1309+
}
1310+
return nil
1311+
})
1312+
if cErr != nil {
1313+
return nil, cErr
12291314
}
12301315

12311316
allRepos = append(allRepos, repos...)
@@ -1239,12 +1324,21 @@ func getRepositoryList(c context.Context, userName string, githubClient *github.
12391324
return allRepos, nil
12401325
}
12411326

1242-
func getRepositoryListByOrg(c context.Context, org string, githubClient *github.Client, opt github.ListOptions) ([]*github.Repository, int, error) {
1327+
func (p *Plugin) getRepositoryListByOrg(c context.Context, ghInfo *GitHubUserInfo, org string, githubClient *github.Client, opt github.ListOptions) ([]*github.Repository, int, error) {
12431328
var allRepos []*github.Repository
12441329
for {
1245-
repos, resp, err := githubClient.Repositories.ListByOrg(c, org, &github.RepositoryListByOrgOptions{Sort: "full_name", ListOptions: opt})
1246-
if err != nil {
1247-
return nil, resp.StatusCode, err
1330+
var repos []*github.Repository
1331+
var resp *github.Response
1332+
var err error
1333+
cErr := p.useGitHubClient(ghInfo, func(info *GitHubUserInfo, token *oauth2.Token) error {
1334+
repos, resp, err = githubClient.Repositories.ListByOrg(c, org, &github.RepositoryListByOrgOptions{Sort: "full_name", ListOptions: opt})
1335+
if err != nil {
1336+
return err
1337+
}
1338+
return nil
1339+
})
1340+
if cErr != nil {
1341+
return nil, resp.StatusCode, cErr
12481342
}
12491343

12501344
allRepos = append(allRepos, repos...)
@@ -1267,7 +1361,7 @@ func (p *Plugin) getRepositories(c *UserContext, w http.ResponseWriter, r *http.
12671361
opt := github.ListOptions{PerPage: 50}
12681362

12691363
if org == "" {
1270-
allRepos, err = getRepositoryList(c.Ctx, "", githubClient, opt)
1364+
allRepos, err = p.getRepositoryList(c.Ctx, c.GHInfo, "", githubClient, opt)
12711365
if err != nil {
12721366
c.Log.WithError(err).Warnf("Failed to list repositories")
12731367
p.writeAPIError(w, &APIErrorResponse{Message: "Failed to fetch repositories", StatusCode: http.StatusInternalServerError})
@@ -1276,10 +1370,10 @@ func (p *Plugin) getRepositories(c *UserContext, w http.ResponseWriter, r *http.
12761370
} else {
12771371
orgsList := p.configuration.getOrganizations()
12781372
for _, org := range orgsList {
1279-
orgRepos, statusCode, err := getRepositoryListByOrg(c.Ctx, org, githubClient, opt)
1373+
orgRepos, statusCode, err := p.getRepositoryListByOrg(c.Ctx, c.GHInfo, org, githubClient, opt)
12801374
if err != nil {
12811375
if statusCode == http.StatusNotFound {
1282-
orgRepos, err = getRepositoryList(c.Ctx, org, githubClient, opt)
1376+
orgRepos, err = p.getRepositoryList(c.Ctx, c.GHInfo, org, githubClient, opt)
12831377
if err != nil {
12841378
c.Log.WithError(err).Warnf("Failed to list repositories", "Organization", org)
12851379
p.writeAPIError(w, &APIErrorResponse{Message: "Failed to fetch repositories", StatusCode: http.StatusInternalServerError})
@@ -1410,14 +1504,21 @@ func (p *Plugin) createIssue(c *UserContext, w http.ResponseWriter, r *http.Requ
14101504
repoName := splittedRepo[1]
14111505

14121506
githubClient := p.githubConnectUser(c.Context.Ctx, c.GHInfo)
1413-
result, resp, err := githubClient.Issues.Create(c.Ctx, owner, repoName, ghIssue)
1414-
if err != nil {
1507+
var resp *github.Response
1508+
var result *github.Issue
1509+
if cErr := p.useGitHubClient(c.GHInfo, func(info *GitHubUserInfo, token *oauth2.Token) error {
1510+
result, resp, err = githubClient.Issues.Create(c.Ctx, owner, repoName, ghIssue)
1511+
if err != nil {
1512+
return err
1513+
}
1514+
return nil
1515+
}); cErr != nil {
14151516
if resp != nil && resp.Response.StatusCode == http.StatusGone {
14161517
p.writeAPIError(w, &APIErrorResponse{ID: "", Message: "Issues are disabled on this repository.", StatusCode: http.StatusMethodNotAllowed})
14171518
return
14181519
}
14191520

1420-
c.Log.WithError(err).Warnf("Failed to create issue")
1521+
c.Log.WithError(cErr).Warnf("Failed to create issue")
14211522
p.writeAPIError(w,
14221523
&APIErrorResponse{
14231524
ID: "",

0 commit comments

Comments
 (0)