diff --git a/internal/pkg/alerts/alert.go b/internal/pkg/alerts/alert.go index 6b9568ff0..924c91df7 100644 --- a/internal/pkg/alerts/alert.go +++ b/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()) } } diff --git a/internal/pkg/alerts/alert_test.go b/internal/pkg/alerts/alert_test.go new file mode 100644 index 000000000..668bec3ea --- /dev/null +++ b/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") +}