From c45b49cb9223840f58a6b366d2a93f75ae7a1420 Mon Sep 17 00:00:00 2001
From: "Grot (@grafanabot)" <43478413+grafanabot@users.noreply.github.com>
Date: Tue, 29 Nov 2022 21:39:41 +0200
Subject: [PATCH] [v9.3.x] Alerting: Fix swallowing of errors when attaching
 images to notifications (#59513)

Alerting: Fix swallowing of errors when attaching images to notifications (#59432)

* Break out image logic and add logging

* Attach alert log context to image attachment

* Fix capitalization

(cherry picked from commit 1481ace52805eed0ca9b04a613131af4c37fa452)

Co-authored-by: Alexander Weaver <weaver.alex.d@gmail.com>
---
 .../ngalert/notifier/channels/pushover.go     | 64 ++++++++++---------
 .../ngalert/notifier/channels/util.go         |  4 +-
 2 files changed, 37 insertions(+), 31 deletions(-)

diff --git a/pkg/services/ngalert/notifier/channels/pushover.go b/pkg/services/ngalert/notifier/channels/pushover.go
index 0df40f44a7f..bb6dc024e04 100644
--- a/pkg/services/ngalert/notifier/channels/pushover.go
+++ b/pkg/services/ngalert/notifier/channels/pushover.go
@@ -209,6 +209,40 @@ func (pn *PushoverNotifier) genPushoverBody(ctx context.Context, as ...*types.Al
 		return nil, b, fmt.Errorf("failed write the message: %w", err)
 	}
 
+	pn.writeImageParts(ctx, w, as...)
+
+	var sound string
+	if status == model.AlertResolved {
+		sound = tmpl(pn.settings.okSound)
+	} else {
+		sound = tmpl(pn.settings.alertingSound)
+	}
+	if sound != "default" {
+		if err := w.WriteField("sound", sound); err != nil {
+			return nil, b, fmt.Errorf("failed to write the sound: %w", err)
+		}
+	}
+
+	// Mark the message as HTML
+	if err := w.WriteField("html", "1"); err != nil {
+		return nil, b, fmt.Errorf("failed to mark the message as HTML: %w", err)
+	}
+	if err := w.Close(); err != nil {
+		return nil, b, fmt.Errorf("failed to close the multipart request: %w", err)
+	}
+
+	if tmplErr != nil {
+		pn.log.Warn("failed to template pushover message", "error", tmplErr.Error())
+	}
+
+	headers := map[string]string{
+		"Content-Type": w.FormDataContentType(),
+	}
+
+	return headers, b, nil
+}
+
+func (pn *PushoverNotifier) writeImageParts(ctx context.Context, w *multipart.Writer, as ...*types.Alert) {
 	// Pushover supports at most one image attachment with a maximum size of pushoverMaxFileSize.
 	// If the image is larger than pushoverMaxFileSize then return an error.
 	_ = withStoredImages(ctx, pn.log, pn.images, func(index int, image ngmodels.Image) error {
@@ -242,34 +276,4 @@ func (pn *PushoverNotifier) genPushoverBody(ctx context.Context, as ...*types.Al
 
 		return ErrImagesDone
 	}, as...)
-
-	var sound string
-	if status == model.AlertResolved {
-		sound = tmpl(pn.settings.okSound)
-	} else {
-		sound = tmpl(pn.settings.alertingSound)
-	}
-	if sound != "default" {
-		if err := w.WriteField("sound", sound); err != nil {
-			return nil, b, fmt.Errorf("failed to write the sound: %w", err)
-		}
-	}
-
-	// Mark the message as HTML
-	if err := w.WriteField("html", "1"); err != nil {
-		return nil, b, fmt.Errorf("failed to mark the message as HTML: %w", err)
-	}
-	if err := w.Close(); err != nil {
-		return nil, b, fmt.Errorf("failed to close the multipart request: %w", err)
-	}
-
-	if tmplErr != nil {
-		pn.log.Warn("failed to template pushover message", "error", tmplErr.Error())
-	}
-
-	headers := map[string]string{
-		"Content-Type": w.FormDataContentType(),
-	}
-
-	return headers, b, nil
 }
diff --git a/pkg/services/ngalert/notifier/channels/util.go b/pkg/services/ngalert/notifier/channels/util.go
index 3d5bcd62e8e..18535f62d3c 100644
--- a/pkg/services/ngalert/notifier/channels/util.go
+++ b/pkg/services/ngalert/notifier/channels/util.go
@@ -82,7 +82,8 @@ func getImage(ctx context.Context, l log.Logger, imageStore ImageStore, alert ty
 // images have been found.
 func withStoredImages(ctx context.Context, l log.Logger, imageStore ImageStore, forEachFunc forEachImageFunc, alerts ...*types.Alert) error {
 	for index, alert := range alerts {
-		img, err := getImage(ctx, l, imageStore, *alert)
+		logger := l.New("alert", alert.String())
+		img, err := getImage(ctx, logger, imageStore, *alert)
 		if err != nil {
 			return err
 		} else if img != nil {
@@ -90,6 +91,7 @@ func withStoredImages(ctx context.Context, l log.Logger, imageStore ImageStore,
 				if errors.Is(err, ErrImagesDone) {
 					return nil
 				}
+				logger.Error("Failed to attach image to notification", "error", err)
 				return err
 			}
 		}