-
Notifications
You must be signed in to change notification settings - Fork 69
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
Feature/detect broken contacts #615
Conversation
12a55b9
to
7deee8b
Compare
senders/telegram/send.go
Outdated
@@ -37,9 +37,15 @@ func (sender *Sender) SendEvents(events moira.NotificationEvents, contact moira. | |||
sender.logger.Debugf("Calling telegram api with chat_id %s and message body %s", contact.Value, message) | |||
chat, err := sender.getChat(contact.Value) | |||
if err != nil { | |||
if ok, errorBrokenContact := checkBrokenContactError(sender.logger, err); ok { |
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.
It's pretty strange to see that we raise an error if ok
is true. Probably we need to invert bool value in return of function and invert condition here?
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.
Additionally: do we really need this bool value? probably we can just return SenderBrokenContactError
if contact is broken and nil otherwise. And this condition will become just
if errorBrokenContact := checkBrokenContactError(sender.logger, err); errorBrokenContact != nil {
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.
Or even better solution. Let's the function checkBrokenContactError
will return SenderBrokenContactError
if contact is broken or original error otherwise. And here we can remove this second if and use just
if err != nil {
return checkBrokenContactError(sender.logger, err)
}
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.
looks good)
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.
fixed
2176ebd
to
3396a9b
Compare
PR Summary
Additional information
Relates #614