-
Notifications
You must be signed in to change notification settings - Fork 166
Removing log spam for every user without connected accounts #951
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
… connected accounts
| 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 | ||
| } |
There was a problem hiding this comment.
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
|
Test server destroyed |
ogi-m
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, no log spam with this build
|
Plugin Spinwick PR #951 🎉 Test server created! Access here: https://github-pr-951-tdhpc.test.mattermost.cloud
The test server was created successfully, but there was an issue installing or enabling the plugin automatically:
You can manually install the plugin:
Future commits will still attempt to automatically update the plugin. Installation ID: Credentials: Posted securely in this Mattermost channel - Look for PR #951 |
Summary
While in other parts of the plugin there is an explicit check on whether the error returned by
getGitHubUserInfois notapiErrorIDNotConnected(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 errorTicket Link
Fixes https://mattermost.atlassian.net/browse/MM-67031
Fixes #941
QA Steps