Merge pull request #1182 from nikolauspschuetz/fix/alert-swallowed-send-errors-949

Log errors returned when sending webhook alerts

internal/pkg/alerts/alert.go

@@ -40,16 +40,23 @@ func SendWebhookAlert(msg string) {
 		msg = fmt.Sprintf("%s : %s", alert_additional_info, msg)
 	}
 
+	var errs []error
 	switch AlertSink(alert_sink) {
 	case AlertSinkSlack:
-		sendSlackAlert(webhook_url, webhook_proxy, msg)
+		errs = sendSlackAlert(webhook_url, webhook_proxy, msg)
 	case AlertSinkTeams:
-		sendTeamsAlert(webhook_url, webhook_proxy, msg)
+		errs = sendTeamsAlert(webhook_url, webhook_proxy, msg)
 	case AlertSinkGoogleChat:
-		sendGoogleChatAlert(webhook_url, webhook_proxy, msg)
+		errs = sendGoogleChatAlert(webhook_url, webhook_proxy, msg)
 	default:
 		msg = strings.ReplaceAll(msg, "*", "")
-		sendRawWebhookAlert(webhook_url, webhook_proxy, msg)
+		errs = sendRawWebhookAlert(webhook_url, webhook_proxy, msg)
+	}
+
+	// Previously the errors returned by the send functions were discarded, so a
+	// failing webhook (e.g. Teams) produced no output at all. Surface them. (#949)
+	for _, err := range errs {
+		logrus.Errorf("Error sending alert: %s", err.Error())
 	}
 }
 

internal/pkg/alerts/alert_test.go

@@ -0,0 +1,40 @@
+package alert
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/sirupsen/logrus"
+	logrustest "github.com/sirupsen/logrus/hooks/test"
+	"github.com/stretchr/testify/assert"
+)
+
+// TestSendWebhookAlert_LogsSendErrors is a regression test for #949: a failing
+// webhook (here, a non-2xx response) previously produced no output at all
+// because the errors returned by the send functions were discarded. They must
+// now be surfaced as error logs.
+func TestSendWebhookAlert_LogsSendErrors(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusInternalServerError)
+	}))
+	defer server.Close()
+
+	hook := logrustest.NewGlobal()
+	defer hook.Reset()
+
+	t.Setenv("ALERT_WEBHOOK_URL", server.URL)
+	t.Setenv("ALERT_SINK", string(AlertSinkTeams))
+
+	SendWebhookAlert("test message")
+
+	var logged bool
+	for _, entry := range hook.AllEntries() {
+		if entry.Level == logrus.ErrorLevel && strings.Contains(entry.Message, "Error sending alert") {
+			logged = true
+			break
+		}
+	}
+	assert.True(t, logged, "expected the swallowed webhook error to be logged")
+}