Fix error handling in template rendering for Telegram #4353
Conversation
Signed-off-by: Mikhail Burshteyn <mdburshteyn@gmail.com>
notify/telegram/telegram.go
Outdated
|
|
||
| messageText, truncated := notify.TruncateInRunes(tmpl(n.conf.Message), maxMessageLenRunes) | ||
| if err != nil { | ||
| return true, err |
There was a problem hiding this comment.
I think this should be false, no? The message cannot be retried as it will fail again for the same reason?
There was a problem hiding this comment.
It will most probably fail again indeed.
However, I see that below in the code true is returned if getBotToken() returns an error. As far as I understand, getBotToken() will not probably recover on its own (only if someone reloads the config or changes the bot_token_file on the filesystem), similarly to the template rendering. This is why I chose true in this case, but I can change this to false if fits better here.
There was a problem hiding this comment.
I think returning true for getBotToken() is also a bug 👍
There was a problem hiding this comment.
Ok, changed this case to false
Signed-off-by: Mikhail Burshteyn <mdburshteyn@gmail.com>
2 participants
notify.TmplTextandnotify.TmplHTMLuse an unusual error handling pattern: the error is not explicitly returned but still must be checked by callers.In the Telegram notifier the check is missing. Because of this, if template rendering fails, Alertmanager still attempts to send an empty message to Telegram and logs an error with text
Bad Request: message text is empty (400).With this patch applied, Alertmanager will log the actual error that happens during template rendering.