Skip to content

Conversation

@avasconcelos114
Copy link

Summary

While in other parts of the plugin there is an explicit check on whether the error returned by getGitHubUserInfo is not apiErrorIDNotConnected (because this would mean the account is simply not connected and we should do an early return instead of treating it as a potential bug), these two cases were logging these are errors without evaluating the ID of the error

Ticket Link

Fixes https://mattermost.atlassian.net/browse/MM-67031
Fixes #941

QA Steps

  1. Log in with a user who has not connected their account to GitHub
  2. Keep tab open running in the background for several minutes (the issue happens due to websocket reconnects and the plugin attempts to refresh sidebar content)
  3. Verify the presence/absence of log lines saying "failed to get GitHub user info"

@avasconcelos114 avasconcelos114 self-assigned this Dec 31, 2025
@avasconcelos114 avasconcelos114 requested a review from a team as a code owner December 31, 2025 09:01
@avasconcelos114 avasconcelos114 added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester labels Dec 31, 2025
Comment on lines 611 to 620
if err != nil && err.ID != apiErrorIDNotConnected {
c.Log.WithError(err).Errorf("failed to get GitHub user info")
p.writeAPIError(w, &APIErrorResponse{Message: fmt.Sprintf("failed to get GitHub user info. %s", err.Message), StatusCode: err.StatusCode})
return
}

if info == nil || info.Token == nil {
p.writeJSON(w, resp)
return
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case we are letting cases where errors with ID == apiErrorIDNotConnected to be returned (as I think they were intended to) by the check on info == nil

@avasconcelos114 avasconcelos114 changed the title fix: removing log noise from printing expected cases of users without… Removing log spam for every user without connected accounts Dec 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error log spam for every active user that does not have a github account connected

2 participants