Error from run_script is not consistently passed to deferred functions

cmd/backup/run_script.go

@@ -63,11 +63,22 @@ func runScript(c *Config) (err error) {
 	}
 
 	if initErr := s.init(); initErr != nil {
+		if hookErr := s.runHooks(initErr); hookErr != nil {
+			err = errwrap.Wrap(
+				nil,
+				fmt.Sprintf(
+					"error %v instantiating script followed by %v calling the registered hooks",
+					initErr,
+					hookErr,
+				),
+			)
+			return
+		}
 		err = errwrap.Wrap(initErr, "error instantiating script")
 		return
 	}
 
-	return func() (err error) {
+	err = func() (err error) {
 		scriptErr := func() error {
 			if err := s.withLabeledCommands(lifecyclePhaseArchive, func() (err error) {
 				restartContainersAndServices, err := s.stopContainersAndServices()
@@ -121,4 +132,6 @@ func runScript(c *Config) (err error) {
 		}
 		return nil
 	}()
+
+	return
 }

cmd/backup/script.go

@@ -78,6 +78,69 @@ func (s *script) init() error {
 		s.stats.TookTime = s.stats.EndTime.Sub(s.stats.StartTime)
 		return nil
 	})
+	// Register notifications first so they can fire in case of other init errors.
+	if s.c.EmailNotificationRecipient != "" {
+		emailURL := fmt.Sprintf(
+			"smtp://%s:%s@%s:%d/?from=%s&to=%s",
+			s.c.EmailSMTPUsername,
+			s.c.EmailSMTPPassword,
+			s.c.EmailSMTPHost,
+			s.c.EmailSMTPPort,
+			s.c.EmailNotificationSender,
+			s.c.EmailNotificationRecipient,
+		)
+		s.c.NotificationURLs = append(s.c.NotificationURLs, emailURL)
+		s.logger.Warn(
+			"Using EMAIL_* keys for providing notification configuration has been deprecated and will be removed in the next major version.",
+		)
+		s.logger.Warn(
+			"Please use NOTIFICATION_URLS instead. Refer to the README for an upgrade guide.",
+		)
+	}
+
+	hookLevel, ok := hookLevels[s.c.NotificationLevel]
+	if !ok {
+		return errwrap.Wrap(nil, fmt.Sprintf("unknown NOTIFICATION_LEVEL %s", s.c.NotificationLevel))
+	}
+	s.hookLevel = hookLevel
+
+	if len(s.c.NotificationURLs) > 0 {
+		sender, senderErr := shoutrrr.CreateSender(s.c.NotificationURLs...)
+		if senderErr != nil {
+			return errwrap.Wrap(senderErr, "error creating sender")
+		}
+		s.sender = sender
+
+		tmpl := template.New("")
+		tmpl.Funcs(templateHelpers)
+		tmpl, err := tmpl.Parse(defaultNotifications)
+		if err != nil {
+			return errwrap.Wrap(err, "unable to parse default notifications templates")
+		}
+
+		if fi, err := os.Stat("/etc/dockervolumebackup/notifications.d"); err == nil && fi.IsDir() {
+			tmpl, err = tmpl.ParseGlob("/etc/dockervolumebackup/notifications.d/*.*")
+			if err != nil {
+				return errwrap.Wrap(err, "unable to parse user defined notifications templates")
+			}
+		}
+		s.template = tmpl
+
+		// To prevent duplicate notifications, ensure the regsistered callbacks
+		// run mutually exclusive.
+		s.registerHook(hookLevelError, func(err error) error {
+			if err == nil {
+				return nil
+			}
+			return s.notifyFailure(err)
+		})
+		s.registerHook(hookLevelInfo, func(err error) error {
+			if err != nil {
+				return nil
+			}
+			return s.notifySuccess()
+		})
+	}
 
 	s.file = path.Join("/tmp", s.c.BackupFilename)
 
@@ -252,68 +315,5 @@ func (s *script) init() error {
 		s.storages = append(s.storages, googleDriveBackend)
 	}
 
-	if s.c.EmailNotificationRecipient != "" {
-		emailURL := fmt.Sprintf(
-			"smtp://%s:%s@%s:%d/?from=%s&to=%s",
-			s.c.EmailSMTPUsername,
-			s.c.EmailSMTPPassword,
-			s.c.EmailSMTPHost,
-			s.c.EmailSMTPPort,
-			s.c.EmailNotificationSender,
-			s.c.EmailNotificationRecipient,
-		)
-		s.c.NotificationURLs = append(s.c.NotificationURLs, emailURL)
-		s.logger.Warn(
-			"Using EMAIL_* keys for providing notification configuration has been deprecated and will be removed in the next major version.",
-		)
-		s.logger.Warn(
-			"Please use NOTIFICATION_URLS instead. Refer to the README for an upgrade guide.",
-		)
-	}
-
-	hookLevel, ok := hookLevels[s.c.NotificationLevel]
-	if !ok {
-		return errwrap.Wrap(nil, fmt.Sprintf("unknown NOTIFICATION_LEVEL %s", s.c.NotificationLevel))
-	}
-	s.hookLevel = hookLevel
-
-	if len(s.c.NotificationURLs) > 0 {
-		sender, senderErr := shoutrrr.CreateSender(s.c.NotificationURLs...)
-		if senderErr != nil {
-			return errwrap.Wrap(senderErr, "error creating sender")
-		}
-		s.sender = sender
-
-		tmpl := template.New("")
-		tmpl.Funcs(templateHelpers)
-		tmpl, err = tmpl.Parse(defaultNotifications)
-		if err != nil {
-			return errwrap.Wrap(err, "unable to parse default notifications templates")
-		}
-
-		if fi, err := os.Stat("/etc/dockervolumebackup/notifications.d"); err == nil && fi.IsDir() {
-			tmpl, err = tmpl.ParseGlob("/etc/dockervolumebackup/notifications.d/*.*")
-			if err != nil {
-				return errwrap.Wrap(err, "unable to parse user defined notifications templates")
-			}
-		}
-		s.template = tmpl
-
-		// To prevent duplicate notifications, ensure the regsistered callbacks
-		// run mutually exclusive.
-		s.registerHook(hookLevelError, func(err error) error {
-			if err == nil {
-				return nil
-			}
-			return s.notifyFailure(err)
-		})
-		s.registerHook(hookLevelInfo, func(err error) error {
-			if err != nil {
-				return nil
-			}
-			return s.notifySuccess()
-		})
-	}
-
 	return nil
 }

test/notifications/run.sh

@@ -46,3 +46,14 @@ if [ "$MESSAGE_BODY" != "Backing up /tmp/test.tar.gz succeeded." ]; then
   fail "Unexpected notification body $MESSAGE_BODY"
 fi
 pass "Custom notification body was used."
+
+NUM_MESSAGES_BEFORE=$(curl -sSL http://admin:custom@localhost:8080/message | jq -r '.messages | length')
+docker compose exec -e AWS_S3_BUCKET_NAME=missing-bucket -e AWS_ACCESS_KEY_ID_FILE=/tmp/missing backup backup \
+  && fail "Expected backup to fail due to missing AWS_ACCESS_KEY_ID_FILE."
+pass "Backup failed with missing AWS_ACCESS_KEY_ID_FILE as expected."
+
+NUM_MESSAGES_AFTER=$(curl -sSL http://admin:custom@localhost:8080/message | jq -r '.messages | length')
+if [ "$NUM_MESSAGES_AFTER" != "$((NUM_MESSAGES_BEFORE + 1))" ]; then
+  fail "Expected one additional notification after failure, got $NUM_MESSAGES_AFTER total."
+fi
+pass "Failure notification was sent."