Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion notify/discord/discord.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error)

alerts := types.Alerts(as...)
data := notify.GetTemplateData(ctx, n.tmpl, as, logger)
tmpl := notify.TmplText(n.tmpl, data, &err)
tmpl := notify.TmplTextWithLogger(n.tmpl, data, &err, logger)
if err != nil {
return false, err
}
Expand Down
2 changes: 1 addition & 1 deletion notify/email/email.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ func (n *Email) Notify(ctx context.Context, as ...*types.Alert) (bool, error) {
var (
tmplErr error
data = notify.GetTemplateData(ctx, n.tmpl, as, n.logger)
tmpl = notify.TmplText(n.tmpl, data, &tmplErr)
tmpl = notify.TmplTextWithLogger(n.tmpl, data, &tmplErr, n.logger)
)
from := tmpl(n.conf.From)
if tmplErr != nil {
Expand Down
2 changes: 1 addition & 1 deletion notify/jira/jira.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error)

tmplTextErr error
data = notify.GetTemplateData(ctx, n.tmpl, as, logger)
tmplText = notify.TmplText(n.tmpl, data, &tmplTextErr)
tmplText = notify.TmplTextWithLogger(n.tmpl, data, &tmplTextErr, logger)
tmplTextFunc = func(tmpl string) (string, error) {
return tmplText(tmpl), tmplTextErr
}
Expand Down
3 changes: 2 additions & 1 deletion notify/mattermost/mattermost.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,8 @@ func (n *Notifier) Notify(ctx context.Context, alert ...*types.Alert) (bool, err
return false, errors.New("webhook url missing")
}

req := n.createRequest(notify.TmplText(n.tmpl, data, &err))
tmplText := notify.TmplTextWithLogger(n.tmpl, data, &err, n.logger)
req := n.createRequest(tmplText)
if err != nil {
return false, err
}
Expand Down
2 changes: 1 addition & 1 deletion notify/msteams/msteams.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error)
logger.Debug("extracted group key")

data := notify.GetTemplateData(ctx, n.tmpl, as, logger)
tmpl := notify.TmplText(n.tmpl, data, &err)
tmpl := notify.TmplTextWithLogger(n.tmpl, data, &err, logger)
if err != nil {
return false, err
}
Expand Down
2 changes: 1 addition & 1 deletion notify/msteamsv2/msteamsv2.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error)
logger.Debug("extracted group key")

data := notify.GetTemplateData(ctx, n.tmpl, as, logger)
tmpl := notify.TmplText(n.tmpl, data, &err)
tmpl := notify.TmplTextWithLogger(n.tmpl, data, &err, logger)
if err != nil {
return false, err
}
Expand Down
2 changes: 1 addition & 1 deletion notify/opsgenie/opsgenie.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ func (n *Notifier) createRequests(ctx context.Context, as ...*types.Alert) ([]*h

data := notify.GetTemplateData(ctx, n.tmpl, as, logger)

tmpl := notify.TmplText(n.tmpl, data, &err)
tmpl := notify.TmplTextWithLogger(n.tmpl, data, &err, logger)

details := make(map[string]string)

Expand Down
6 changes: 3 additions & 3 deletions notify/pagerduty/pagerduty.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ func (n *Notifier) notifyV1(
details map[string]any,
) (bool, error) {
var tmplErr error
tmpl := notify.TmplText(n.tmpl, data, &tmplErr)
tmpl := notify.TmplTextWithLogger(n.tmpl, data, &tmplErr, n.logger)

description, truncated := notify.TruncateInRunes(tmpl(n.conf.Description), maxV1DescriptionLenRunes)
if truncated {
Expand Down Expand Up @@ -211,7 +211,7 @@ func (n *Notifier) notifyV2(
details map[string]any,
) (bool, error) {
var tmplErr error
tmpl := notify.TmplText(n.tmpl, data, &tmplErr)
tmpl := notify.TmplTextWithLogger(n.tmpl, data, &tmplErr, n.logger)

if n.conf.Severity == "" {
n.conf.Severity = "error"
Expand Down Expand Up @@ -367,7 +367,7 @@ func (n *Notifier) renderDetails(
) (map[string]any, error) {
var (
tmplTextErr error
tmplText = notify.TmplText(n.tmpl, data, &tmplTextErr)
tmplText = notify.TmplTextWithLogger(n.tmpl, data, &tmplTextErr, n.logger)
tmplTextFunc = func(tmpl string) (string, error) {
return tmplText(tmpl), tmplTextErr
}
Expand Down
3 changes: 2 additions & 1 deletion notify/pagerduty/pagerduty_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -875,7 +875,8 @@ func TestRenderDetails(t *testing.T) {
conf: &config.PagerdutyConfig{
Details: tt.args.details,
},
tmpl: test.CreateTmpl(t),
tmpl: test.CreateTmpl(t),
logger: promslog.NewNopLogger(),
}
got, err := n.renderDetails(tt.args.data)
if (err != nil) != tt.wantErr {
Expand Down
4 changes: 2 additions & 2 deletions notify/pushover/pushover.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error)
err error
message string
)
tmpl := notify.TmplText(n.tmpl, data, &err)
tmplHTML := notify.TmplHTML(n.tmpl, data, &err)
tmpl := notify.TmplTextWithLogger(n.tmpl, data, &err, logger)
tmplHTML := notify.TmplHTMLWithLogger(n.tmpl, data, &err, logger)

var (
token string
Expand Down
2 changes: 1 addition & 1 deletion notify/rocketchat/rocketchat.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error)
logger.Debug("extracted group key")

data := notify.GetTemplateData(ctx, n.tmpl, as, logger)
tmplText := notify.TmplText(n.tmpl, data, &err)
tmplText := notify.TmplTextWithLogger(n.tmpl, data, &err, logger)
if err != nil {
return false, err
}
Expand Down
2 changes: 1 addition & 1 deletion notify/slack/slack.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error)

var (
data = notify.GetTemplateData(ctx, n.tmpl, as, logger)
tmplText = notify.TmplText(n.tmpl, data, &err)
tmplText = notify.TmplTextWithLogger(n.tmpl, data, &err, logger)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Return template errors before sending the Slack request.

TmplTextWithLogger records failures in err, but Notify never checks err after rendering the request fields. Add an if err != nil { return false, err } check after the final template call and before JSON encoding/network delivery; otherwise a failure short-circuits subsequent fields to empty strings while the malformed request is still sent.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@notify/slack/slack.go` at line 68, In Notify, check err immediately after the
final TmplTextWithLogger call and before JSON encoding or Slack delivery; return
false and err when rendering fails, while preserving the existing request flow
when err is nil.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks like a pre-existing issue, but would be nice to adress @mihir-dixit2k27

)
var markdownIn []string

Expand Down
2 changes: 1 addition & 1 deletion notify/sns/sns.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func (n *Notifier) Notify(ctx context.Context, alert ...*types.Alert) (bool, err
var (
tmplErr error
data = notify.GetTemplateData(ctx, n.tmpl, alert, n.logger)
tmpl = notify.TmplText(n.tmpl, data, &tmplErr)
tmpl = notify.TmplTextWithLogger(n.tmpl, data, &tmplErr, n.logger)
)

client, err := n.createSNSClient(ctx, tmpl, &tmplErr)
Expand Down
4 changes: 2 additions & 2 deletions notify/telegram/telegram.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,14 +75,14 @@ func (n *Notifier) Notify(ctx context.Context, alert ...*types.Alert) (bool, err
var (
err error
data = notify.GetTemplateData(ctx, n.tmpl, alert, logger)
tmpl = notify.TmplText(n.tmpl, data, &err)
tmpl = notify.TmplTextWithLogger(n.tmpl, data, &err, logger)
messageText string
truncated bool
)

switch n.conf.ParseMode {
case "HTML":
tmpl = notify.TmplHTML(n.tmpl, data, &err)
tmpl = notify.TmplHTMLWithLogger(n.tmpl, data, &err, logger)
messageText = tmpl(n.conf.Message)
if err != nil {
return false, err
Expand Down
38 changes: 38 additions & 0 deletions notify/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,44 @@ func TmplHTML(tmpl *template.Template, data *template.Data, err *error) func(str
}
}

// TmplTextWithLogger is like TmplText but additionally logs a warning whenever
// a template fails to render. This makes template errors visible in Alertmanager
// logs even when the calling notifier continues to deliver the notification with
// the partially rendered output.
func TmplTextWithLogger(tmpl *template.Template, data *template.Data, err *error, logger *slog.Logger) func(string) string {
return func(name string) (s string) {
if *err != nil {
return s
}
var tmplErr error
s, tmplErr = tmpl.ExecuteTextString(name, data)
if tmplErr != nil {
logger.Warn("template execution failed", "template", name, "err", tmplErr)
*err = tmplErr
}
return s
}
}

// TmplHTMLWithLogger is like TmplHTML but additionally logs a warning whenever
// a template fails to render. This makes template errors visible in Alertmanager
// logs even when the calling notifier continues to deliver the notification with
// the partially rendered output.
func TmplHTMLWithLogger(tmpl *template.Template, data *template.Data, err *error, logger *slog.Logger) func(string) string {
return func(name string) (s string) {
if *err != nil {
return s
}
var tmplErr error
s, tmplErr = tmpl.ExecuteHTMLString(name, data)
if tmplErr != nil {
logger.Warn("template execution failed", "template", name, "err", tmplErr)
*err = tmplErr
}
return s
}
}

// Key is a string that can be hashed.
type Key string

Expand Down
50 changes: 50 additions & 0 deletions notify/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -273,3 +273,53 @@ func TestGetFailureReasonFromStatusCode(t *testing.T) {
})
}
}

func TestTmplTextWithLogger_LogsOnError(t *testing.T) {
tmpl, err := template.FromGlobs([]string{})
require.NoError(t, err)

data := &template.Data{}
var tmplErr error

var buf bytes.Buffer
logger := promslog.New(&promslog.Config{Writer: &buf})

tmplFn := TmplTextWithLogger(tmpl, data, &tmplErr, logger)

// An unclosed action causes a parse/execute error.
result := tmplFn("{{ .Missing")

require.Error(t, tmplErr)
require.Empty(t, result)
logOutput := buf.String()
require.Contains(t, logOutput, "template execution failed")
require.Contains(t, logOutput, "{{ .Missing") // template name is logged
require.Contains(t, logOutput, tmplErr.Error()) // rendering error is logged

// Subsequent calls must short-circuit without logging again.
buf.Reset()
_ = tmplFn("{{ .AnotherField }}")
require.Empty(t, buf.String(), "no second log expected after first error")
}

func TestTmplHTMLWithLogger_LogsOnError(t *testing.T) {
tmpl, err := template.FromGlobs([]string{})
require.NoError(t, err)

data := &template.Data{}
var tmplErr error

var buf bytes.Buffer
logger := promslog.New(&promslog.Config{Writer: &buf})

tmplFn := TmplHTMLWithLogger(tmpl, data, &tmplErr, logger)

result := tmplFn("{{ .Missing")

require.Error(t, tmplErr)
require.Empty(t, result)
logOutput := buf.String()
require.Contains(t, logOutput, "template execution failed")
require.Contains(t, logOutput, "{{ .Missing") // template name is logged
require.Contains(t, logOutput, tmplErr.Error()) // rendering error is logged
}
4 changes: 2 additions & 2 deletions notify/victorops/victorops.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error)
var err error
var (
data = notify.GetTemplateData(ctx, n.tmpl, as, n.logger)
tmpl = notify.TmplText(n.tmpl, data, &err)
tmpl = notify.TmplTextWithLogger(n.tmpl, data, &err, n.logger)
apiURL = n.conf.APIURL.Copy()
)

Expand Down Expand Up @@ -125,7 +125,7 @@ func (n *Notifier) createVictorOpsPayload(ctx context.Context, as ...*types.Aler
var (
alerts = types.Alerts(as...)
data = notify.GetTemplateData(ctx, n.tmpl, as, n.logger)
tmpl = notify.TmplText(n.tmpl, data, &err)
tmpl = notify.TmplTextWithLogger(n.tmpl, data, &err, n.logger)

messageType = tmpl(n.conf.MessageType)
stateMessage = tmpl(n.conf.StateMessage)
Expand Down
2 changes: 1 addition & 1 deletion notify/webex/webex.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error)
logger.Debug("extracted group key")

data := notify.GetTemplateData(ctx, n.tmpl, as, logger)
tmpl := notify.TmplText(n.tmpl, data, &err)
tmpl := notify.TmplTextWithLogger(n.tmpl, data, &err, logger)
if err != nil {
return false, err
}
Expand Down
4 changes: 2 additions & 2 deletions notify/webhook/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ func (n *Notifier) Notify(ctx context.Context, alerts ...*types.Alert) (bool, er

var url string
var tmplErr error
tmpl := notify.TmplText(n.tmpl, data, &tmplErr)
tmpl := notify.TmplTextWithLogger(n.tmpl, data, &tmplErr, logger)

if n.conf.URL != "" {
url = tmpl(string(n.conf.URL))
Expand Down Expand Up @@ -157,7 +157,7 @@ func (n *Notifier) renderPayload(
) (bytes.Buffer, error) {
var (
tmplTextErr error
tmplText = notify.TmplText(n.tmpl, data.Data, &tmplTextErr)
tmplText = notify.TmplTextWithLogger(n.tmpl, data.Data, &tmplTextErr, n.logger)
tmplTextFunc = func(tmpl string) (string, error) {
return tmplText(tmpl), tmplTextErr
}
Expand Down
2 changes: 1 addition & 1 deletion notify/wechat/wechat.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error)

data := notify.GetTemplateData(ctx, n.tmpl, as, logger)

tmpl := notify.TmplText(n.tmpl, data, &err)
tmpl := notify.TmplTextWithLogger(n.tmpl, data, &err, logger)
if err != nil {
return false, err
}
Expand Down
Loading