removeedd waitgroup, using sync/errgroup instead (to pass error messages back). returning errors in test notifications endpoint payload. Adding failure tests for webhooks, scripts & shoutrr.

go.mod

@@ -18,5 +18,6 @@ require (
 	github.com/stretchr/testify v1.5.1
 	github.com/urfave/cli/v2 v2.2.0
 	golang.org/x/crypto v0.0.0-20200323165209-0ec3e9974c59 // indirect
+	golang.org/x/sync v0.0.0-20190423024810-112230192c58
 	gopkg.in/yaml.v2 v2.3.0 // indirect
 )

go.sum

@@ -347,6 +347,7 @@ golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJ
 golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20190227155943-e225da77a7e6/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
+golang.org/x/sync v0.0.0-20190423024810-112230192c58 h1:8gQV6CLnAEikrhgkHFbMAEhagSSnXWGV915qUMm9mrU=
 golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sys v0.0.0-20180823144017-11551d06cbcc/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=

webapp/backend/pkg/notify/notify.go

@@ -3,17 +3,18 @@ package notify
 import (
 	"bytes"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"github.com/analogj/go-util/utils"
 	"github.com/analogj/scrutiny/webapp/backend/pkg/config"
 	"github.com/containrrr/shoutrrr"
 	shoutrrrTypes "github.com/containrrr/shoutrrr/pkg/types"
 	"github.com/sirupsen/logrus"
+	"golang.org/x/sync/errgroup"
 	"net/http"
 	"net/url"
 	"os"
 	"strings"
-	"sync"
 	"time"
 )
 
@@ -102,62 +103,65 @@ func (n *Notify) Send() error {
 	n.Logger.Debugf("Configured shoutrrr: %v", notifyShoutrrr)
 
 	//run all scripts, webhooks and shoutrr commands in parallel
-	var wg sync.WaitGroup
+	//var wg sync.WaitGroup
+	var eg errgroup.Group
 
 	for _, notifyWebhook := range notifyWebhooks {
 		// execute collection in parallel go-routines
-		wg.Add(1)
+		eg.Go(func() error { return n.SendWebhookNotification(notifyWebhook) })
-		go n.SendWebhookNotification(&wg, notifyWebhook)
 	}
 	for _, notifyScript := range notifyScripts {
 		// execute collection in parallel go-routines
-		wg.Add(1)
+		eg.Go(func() error { return n.SendScriptNotification(notifyScript) })
-		go n.SendScriptNotification(&wg, notifyScript)
 	}
 	for _, shoutrrrUrl := range notifyShoutrrr {
-		wg.Add(1)
+		eg.Go(func() error { return n.SendShoutrrrNotification(shoutrrrUrl) })
-		go n.SendShoutrrrNotification(&wg, shoutrrrUrl)
 	}
 
 	//and wait for completion, error or timeout.
 	n.Logger.Debugf("Main: waiting for notifications to complete.")
-	//wg.Wait()
+
-	if waitTimeout(&wg, time.Minute) { //wait for 1 minute
+	if err := eg.Wait(); err == nil {
-		fmt.Println("Timed out while sending notifications")
+		n.Logger.Info("Successfully sent notifications. Check logs for more information.")
+		return nil
 	} else {
-		fmt.Println("Sent notifications. Check logs for more information.")
+		n.Logger.Error("One or more notifications failed to send successfully. See logs for more information.")
+		return err
 	}
-	return nil
+	////wg.Wait()
+	//if waitTimeout(&wg, time.Minute) { //wait for 1 minute
+	//	fmt.Println("Timed out while sending notifications")
+	//} else {
+	//}
+	//return nil
 }
 
-func (n *Notify) SendWebhookNotification(wg *sync.WaitGroup, webhookUrl string) {
+func (n *Notify) SendWebhookNotification(webhookUrl string) error {
-	defer wg.Done()
 	n.Logger.Infof("Sending Webhook to %s", webhookUrl)
 	requestBody, err := json.Marshal(n.Payload)
 	if err != nil {
 		n.Logger.Errorf("An error occurred while sending Webhook to %s: %v", webhookUrl, err)
-		return
+		return err
 	}
 
 	resp, err := http.Post(webhookUrl, "application/json", bytes.NewBuffer(requestBody))
 	if err != nil {
 		n.Logger.Errorf("An error occurred while sending Webhook to %s: %v", webhookUrl, err)
-		return
+		return err
 	}
 	defer resp.Body.Close()
 	//we don't care about resp body content, but maybe we should log it?
+	return nil
 }
 
-func (n *Notify) SendScriptNotification(wg *sync.WaitGroup, scriptUrl string) {
+func (n *Notify) SendScriptNotification(scriptUrl string) error {
-	defer wg.Done()
-
 	//check if the script exists.
 	scriptPath := strings.TrimPrefix(scriptUrl, "script://")
 	n.Logger.Infof("Executing Script %s", scriptPath)
 
 	if !utils.FileExists(scriptPath) {
 		n.Logger.Errorf("Script does not exist: %s", scriptPath)
-		return
+		return errors.New(fmt.Sprintf("custom script path does not exist: %s", scriptPath))
 	}
 
 	copyEnv := os.Environ()
@@ -171,20 +175,20 @@ func (n *Notify) SendScriptNotification(wg *sync.WaitGroup, scriptUrl string) {
 	err := utils.CmdExec(scriptPath, []string{}, "", copyEnv, "")
 	if err != nil {
 		n.Logger.Errorf("An error occurred while executing script %s: %v", scriptPath, err)
+		return err
 	}
-	return
+	return nil
 }
 
-func (n *Notify) SendShoutrrrNotification(wg *sync.WaitGroup, shoutrrrUrl string) {
+func (n *Notify) SendShoutrrrNotification(shoutrrrUrl string) error {
 
 	fmt.Printf("Sending Notifications to %v", shoutrrrUrl)
 	n.Logger.Infof("Sending notifications to %v", shoutrrrUrl)
 
-	defer wg.Done()
 	sender, err := shoutrrr.CreateSender(shoutrrrUrl)
 	if err != nil {
 		n.Logger.Errorf("An error occurred while sending notifications %v: %v", shoutrrrUrl, err)
-		return
+		return err
 	}
 
 	//sender.SetLogger(n.Logger.)
@@ -193,15 +197,21 @@ func (n *Notify) SendShoutrrrNotification(wg *sync.WaitGroup, shoutrrrUrl string
 
 	if err != nil {
 		n.Logger.Errorf("An error occurred  occurred while generating notification payload for %s:\n %v", serviceName, shoutrrrUrl, err)
+		return err
 	}
 
 	errs := sender.Send(n.Payload.Message, params)
 	if len(errs) > 0 {
-		n.Logger.Errorf("One or more errors occurred  occurred while sending notifications for %s:\n %v", shoutrrrUrl, errs)
+		n.Logger.Errorf("One or more errors occurred  occurred while sending notifications for %s:", shoutrrrUrl)
+		var errstrings []string
+
 		for _, err := range errs {
 			n.Logger.Error(err)
+			errstrings = append(errstrings, err.Error())
 		}
+		return errors.New(strings.Join(errstrings, "\n"))
 	}
+	return nil
 }
 
 func (n *Notify) GenShoutrrrNotificationParams(shoutrrrUrl string) (string, *shoutrrrTypes.Params, error) {
@@ -243,20 +253,3 @@ func (n *Notify) GenShoutrrrNotificationParams(shoutrrrUrl string) (string, *sho
 
 	return serviceName, params, nil
 }
-
-//utility functions
-// waitTimeout waits for the waitgroup for the specified max timeout.
-// Returns true if waiting timed out.
-func waitTimeout(wg *sync.WaitGroup, timeout time.Duration) bool {
-	c := make(chan struct{})
-	go func() {
-		defer close(c)
-		wg.Wait()
-	}()
-	select {
-	case <-c:
-		return false // completed normally
-	case <-time.After(timeout):
-		return true // timed out
-	}
-}

webapp/backend/pkg/web/handler/send_test_notification.go

@@ -30,6 +30,7 @@ func SendTestNotification(c *gin.Context) {
 		logger.Errorln("An error occurred while sending test notification", err)
 		c.JSON(http.StatusInternalServerError, gin.H{
 			"success": false,
+			"errors":  []string{err.Error()},
 		})
 	} else {
 		c.JSON(http.StatusOK, dbModels.DeviceWrapper{

webapp/backend/pkg/web/server_test.go

@@ -188,6 +188,78 @@ func TestSendTestNotificationRoute(t *testing.T) {
 	require.Equal(t, 200, wr.Code)
 }
 
+func TestSendTestNotificationRoute_WebhookFailure(t *testing.T) {
+	//setup
+	parentPath, _ := ioutil.TempDir("", "")
+	defer os.RemoveAll(parentPath)
+	mockCtrl := gomock.NewController(t)
+	defer mockCtrl.Finish()
+	fakeConfig := mock_config.NewMockInterface(mockCtrl)
+	fakeConfig.EXPECT().GetString("web.database.location").AnyTimes().Return(path.Join(parentPath, "scrutiny_test.db"))
+	fakeConfig.EXPECT().GetString("web.src.frontend.path").AnyTimes().Return(parentPath)
+	fakeConfig.EXPECT().GetStringSlice("notify.urls").AnyTimes().Return([]string{"https://unroutable.domain.example.asdfghj"})
+	ae := web.AppEngine{
+		Config: fakeConfig,
+	}
+	router := ae.Setup(logrus.New())
+
+	//test
+	wr := httptest.NewRecorder()
+	req, _ := http.NewRequest("POST", "/api/health/notify", strings.NewReader("{}"))
+	router.ServeHTTP(wr, req)
+
+	//assert
+	require.Equal(t, 500, wr.Code)
+}
+
+func TestSendTestNotificationRoute_ScriptFailure(t *testing.T) {
+	//setup
+	parentPath, _ := ioutil.TempDir("", "")
+	defer os.RemoveAll(parentPath)
+	mockCtrl := gomock.NewController(t)
+	defer mockCtrl.Finish()
+	fakeConfig := mock_config.NewMockInterface(mockCtrl)
+	fakeConfig.EXPECT().GetString("web.database.location").AnyTimes().Return(path.Join(parentPath, "scrutiny_test.db"))
+	fakeConfig.EXPECT().GetString("web.src.frontend.path").AnyTimes().Return(parentPath)
+	fakeConfig.EXPECT().GetStringSlice("notify.urls").AnyTimes().Return([]string{"script:///missing/path/on/disk"})
+	ae := web.AppEngine{
+		Config: fakeConfig,
+	}
+	router := ae.Setup(logrus.New())
+
+	//test
+	wr := httptest.NewRecorder()
+	req, _ := http.NewRequest("POST", "/api/health/notify", strings.NewReader("{}"))
+	router.ServeHTTP(wr, req)
+
+	//assert
+	require.Equal(t, 500, wr.Code)
+}
+
+func TestSendTestNotificationRoute_ShoutrrrFailure(t *testing.T) {
+	//setup
+	parentPath, _ := ioutil.TempDir("", "")
+	defer os.RemoveAll(parentPath)
+	mockCtrl := gomock.NewController(t)
+	defer mockCtrl.Finish()
+	fakeConfig := mock_config.NewMockInterface(mockCtrl)
+	fakeConfig.EXPECT().GetString("web.database.location").AnyTimes().Return(path.Join(parentPath, "scrutiny_test.db"))
+	fakeConfig.EXPECT().GetString("web.src.frontend.path").AnyTimes().Return(parentPath)
+	fakeConfig.EXPECT().GetStringSlice("notify.urls").AnyTimes().Return([]string{"discord://invalidtoken@channel"})
+	ae := web.AppEngine{
+		Config: fakeConfig,
+	}
+	router := ae.Setup(logrus.New())
+
+	//test
+	wr := httptest.NewRecorder()
+	req, _ := http.NewRequest("POST", "/api/health/notify", strings.NewReader("{}"))
+	router.ServeHTTP(wr, req)
+
+	//assert
+	require.Equal(t, 500, wr.Code)
+}
+
 func TestGetDevicesSummaryRoute_Nvme(t *testing.T) {
 	//setup
 	parentPath, _ := ioutil.TempDir("", "")