Improve notifications and module failures and mirror them

modules/modules.go

@@ -40,6 +40,7 @@ type Module struct { //nolint:maligned // not worth the effort
 	// failure status
 	failureStatus uint8
 	failureID     string
+	failureTitle  string
 	failureMsg    string
 
 	// lifecycle callback functions
@@ -62,7 +63,7 @@ type Module struct { //nolint:maligned // not worth the effort
 	waitGroup    sync.WaitGroup
 
 	// events
-	eventHooks     map[string][]*eventHook
+	eventHooks     map[string]*eventHooks
 	eventHooksLock sync.RWMutex
 
 	// dependency mgmt
@@ -127,8 +128,9 @@ func (m *Module) prep(reports chan *report) {
 		// set status
 		if err != nil {
 			m.Error(
-				"module-failed-prep",
-				fmt.Sprintf("failed to prep module: %s", err.Error()),
+				fmt.Sprintf("%s:prep-failed", m.Name),
+				fmt.Sprintf("Preparing module %s failed", m.Name),
+				fmt.Sprintf("Failed to prep module: %s", err.Error()),
 			)
 		} else {
 			m.Lock()
@@ -183,8 +185,9 @@ func (m *Module) start(reports chan *report) {
 		// set status
 		if err != nil {
 			m.Error(
-				"module-failed-start",
-				fmt.Sprintf("failed to start module: %s", err.Error()),
+				fmt.Sprintf("%s:start-failed", m.Name),
+				fmt.Sprintf("Starting module %s failed", m.Name),
+				fmt.Sprintf("Failed to start module: %s", err.Error()),
 			)
 		} else {
 			m.Lock()
@@ -270,8 +273,9 @@ func (m *Module) stopAllTasks(reports chan *report) {
 	// set status
 	if err != nil {
 		m.Error(
-			"module-failed-stop",
-			fmt.Sprintf("failed to stop module: %s", err.Error()),
+			fmt.Sprintf("%s:stop-failed", m.Name),
+			fmt.Sprintf("Stopping module %s failed", m.Name),
+			fmt.Sprintf("Failed to stop module: %s", err.Error()),
 		)
 	}
 
@@ -328,7 +332,7 @@ func initNewModule(name string, prep, start, stop func() error, dependencies ...
 		taskCnt:             &taskCnt,
 		microTaskCnt:        &microTaskCnt,
 		waitGroup:           sync.WaitGroup{},
-		eventHooks:          make(map[string][]*eventHook),
+		eventHooks:          make(map[string]*eventHooks),
 		depNames:            dependencies,
 	}
 

modules/status.go

@@ -1,5 +1,11 @@
 package modules
 
+import (
+	"context"
+
+	"github.com/tevino/abool"
+)
+
 // Module Status Values
 const (
 	StatusDead      uint8 = 0 // not prepared, not started
@@ -25,6 +31,23 @@ const (
 	statusNothingToDo
 )
 
+var (
+	failureUpdateNotifyFunc        func(moduleFailure uint8, id, title, msg string)
+	failureUpdateNotifyFuncEnabled = abool.NewBool(false)
+	failureUpdateNotifyFuncReady   = abool.NewBool(false)
+)
+
+// SetFailureUpdateNotifyFunc sets a function that is called on every change
+// of a module's failure status.
+func SetFailureUpdateNotifyFunc(fn func(moduleFailure uint8, id, title, msg string)) bool {
+	if failureUpdateNotifyFuncEnabled.SetToIf(false, true) {
+		failureUpdateNotifyFunc = fn
+		failureUpdateNotifyFuncReady.Set()
+		return true
+	}
+	return false
+}
+
 // Online returns whether the module is online.
 func (m *Module) Online() bool {
 	return m.Status() == StatusOnline
@@ -57,38 +80,52 @@ func (m *Module) FailureStatus() (failureStatus uint8, failureID, failureMsg str
 }
 
 // Hint sets failure status to hint. This is a somewhat special failure status, as the module is believed to be working correctly, but there is an important module specific information to convey. The supplied failureID is for improved automatic handling within connected systems, the failureMsg is for humans.
-func (m *Module) Hint(failureID, failureMsg string) {
+func (m *Module) Hint(id, title, msg string) {
 	m.Lock()
 	defer m.Unlock()
 
-	m.failureStatus = FailureHint
-	m.failureID = failureID
-	m.failureMsg = failureMsg
-
-	m.notifyOfChange()
+	m.setFailure(FailureHint, id, title, msg)
 }
 
 // Warning sets failure status to warning. The supplied failureID is for improved automatic handling within connected systems, the failureMsg is for humans.
-func (m *Module) Warning(failureID, failureMsg string) {
+func (m *Module) Warning(id, title, msg string) {
 	m.Lock()
 	defer m.Unlock()
 
-	m.failureStatus = FailureWarning
-	m.failureID = failureID
-	m.failureMsg = failureMsg
-
-	m.notifyOfChange()
+	m.setFailure(FailureWarning, id, title, msg)
 }
 
 // Error sets failure status to error. The supplied failureID is for improved automatic handling within connected systems, the failureMsg is for humans.
-func (m *Module) Error(failureID, failureMsg string) {
+func (m *Module) Error(id, title, msg string) {
 	m.Lock()
 	defer m.Unlock()
 
-	m.failureStatus = FailureError
-	m.failureID = failureID
-	m.failureMsg = failureMsg
+	m.setFailure(FailureError, id, title, msg)
+}
 
+func (m *Module) setFailure(status uint8, id, title, msg string) {
+	// Send an update before we override a previous failure.
+	if failureUpdateNotifyFuncReady.IsSet() && m.failureID != "" {
+		updateFailureID := m.failureID
+		m.StartWorker("failure status updater", func(context.Context) error {
+			// Only use data in worker that won't change anymore.
+			failureUpdateNotifyFunc(FailureNone, updateFailureID, "", "")
+			return nil
+		})
+	}
+
+	m.failureStatus = status
+	m.failureID = id
+	m.failureTitle = title
+	m.failureMsg = msg
+
+	if failureUpdateNotifyFuncReady.IsSet() {
+		m.StartWorker("failure status updater", func(context.Context) error {
+			// Only use data in worker that won't change anymore.
+			failureUpdateNotifyFunc(status, id, title, msg)
+			return nil
+		})
+	}
 	m.notifyOfChange()
 }
 
@@ -98,12 +135,24 @@ func (m *Module) Resolve(failureID string) {
 	defer m.Unlock()
 
 	if failureID == "" || failureID == m.failureID {
+		// Propagate resolving.
+		if failureUpdateNotifyFuncReady.IsSet() {
+			updateFailureID := m.failureID
+			m.StartWorker("failure status updater", func(context.Context) error {
+				// Only use data in worker that won't change anymore.
+				failureUpdateNotifyFunc(FailureNone, updateFailureID, "", "")
+				return nil
+			})
+		}
+
+		// Set failure status on module.
 		m.failureStatus = FailureNone
 		m.failureID = ""
+		m.failureTitle = ""
 		m.failureMsg = ""
-	}
 
-	m.notifyOfChange()
+		m.notifyOfChange()
+	}
 }
 
 // readyToPrep returns whether all dependencies are ready for this module to prep.

modules/subsystems/module.go

@@ -76,7 +76,8 @@ func prep() error {
 			if err != nil {
 				module.Error(
 					"modulemgmt-failed",
-					fmt.Sprintf("The subsystem framework failed to start or stop one or more modules.\nError: %s\nCheck logs for more information.", err),
+					"A Module failed to start",
+					fmt.Sprintf("The subsystem framework failed to start or stop one or more modules.\nError: %s\nCheck logs for more information or try to restart.", err),
 				)
 				return nil
 			}

modules/subsystems/subsystems_test.go

@@ -77,7 +77,7 @@ func TestSubsystems(t *testing.T) {
 	// test
 
 	// let module fail
-	feature1.Error("test-fail", "Testing Fail")
+	feature1.Error("test-fail", "Test Fail", "Testing Fail")
 	time.Sleep(10 * time.Millisecond)
 	if sub1.FailureStatus != modules.FailureError {
 		t.Fatal("error did not propagate")

notifications/module-mirror.go

@@ -0,0 +1,95 @@
+package notifications
+
+import (
+	"github.com/safing/portbase/log"
+	"github.com/safing/portbase/modules"
+)
+
+// AttachToModule attaches the notification to a module and changes to the
+// notification will be reflected on the module failure status.
+func (n *Notification) AttachToModule(m *modules.Module) {
+	log.Errorf("notifications: attaching %q", n.EventID)
+
+	if m == nil {
+		log.Warningf("notifications: cannot remove attached module from notification %s", n.EventID)
+		return
+	}
+
+	n.lock.Lock()
+	defer n.lock.Unlock()
+
+	if n.State != Active {
+		log.Warningf("notifications: cannot attach module to inactive notification %s", n.EventID)
+		return
+	}
+	if n.belongsTo != nil {
+		log.Warningf("notifications: cannot override attached module for notification %s", n.EventID)
+		return
+	}
+
+	// Attach module.
+	n.belongsTo = m
+
+	// Set module failure status.
+	switch n.Type { //nolint:exhaustive
+	case Info:
+		m.Hint(n.EventID, n.Title, n.Message)
+	case Warning:
+		m.Warning(n.EventID, n.Title, n.Message)
+	case Error:
+		m.Error(n.EventID, n.Title, n.Message)
+	default:
+		log.Warningf("notifications: incompatible type for attaching to module in notification %s", n.EventID)
+		m.Error(n.EventID, n.Title, n.Message+" [incompatible notification type]")
+	}
+}
+
+// resolveModuleFailure removes the notification from the module failure status.
+func (n *Notification) resolveModuleFailure() {
+	log.Errorf("notifications: resolving %q", n.EventID)
+
+	if n.belongsTo != nil {
+		// Resolve failure in attached module.
+		n.belongsTo.Resolve(n.EventID)
+
+		// Reset attachment in order to mitigate duplicate failure resolving.
+		// Re-attachment is prevented by the state check when attaching.
+		n.belongsTo = nil
+	}
+}
+
+func init() {
+	modules.SetFailureUpdateNotifyFunc(mirrorModuleStatus)
+}
+
+func mirrorModuleStatus(moduleFailure uint8, id, title, msg string) {
+	log.Errorf("notifications: mirroring %d %q %q %q", moduleFailure, id, title, msg)
+
+	// Ignore "resolve all" requests.
+	if id == "" {
+		return
+	}
+
+	// Get notification from storage.
+	n, ok := getNotification(id)
+	if ok {
+		// The notification already exists.
+
+		// Check if we should delete it.
+		if moduleFailure == modules.FailureNone {
+			n.Delete()
+		}
+
+		return
+	}
+
+	// A notification for the given ID does not yet exists, create it.
+	switch moduleFailure {
+	case modules.FailureHint:
+		NotifyInfo(id, title, msg)
+	case modules.FailureWarning:
+		NotifyWarn(id, title, msg)
+	case modules.FailureError:
+		NotifyError(id, title, msg)
+	}
+}

notifications/notification.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/safing/portbase/database/record"
 	"github.com/safing/portbase/log"
+	"github.com/safing/portbase/modules"
 	"github.com/safing/portbase/utils"
 )
 
@@ -19,6 +20,7 @@ const (
 	Info    Type = 0
 	Warning Type = 1
 	Prompt  Type = 2
+	Error   Type = 3
 )
 
 // State describes the state of a notification.
@@ -70,6 +72,9 @@ type Notification struct {
 	// of the notification is available. Note that the message should already
 	// have any paramerized values replaced.
 	Message string
+	// ShowOnSystem specifies if the notification should be also shown on the
+	// operating system.
+	ShowOnSystem bool
 	// EventData contains an additional payload for the notification. This payload
 	// may contain contextual data and may be used by a localization framework
 	// to populate the notification message template.
@@ -91,6 +96,10 @@ type Notification struct {
 	// based on the user selection.
 	SelectedActionID string
 
+	// belongsTo holds the module this notification belongs to. The notification
+	// lifecycle will be mirrored to the module's failure status.
+	belongsTo *modules.Module
+
 	lock           sync.Mutex
 	actionFunction NotificationActionFn // call function to process action
 	actionTrigger  chan string          // and/or send to a channel
@@ -99,8 +108,53 @@ type Notification struct {
 
 // Action describes an action that can be taken for a notification.
 type Action struct {
-	ID   string
-	Text string
+	ID      string
+	Text    string
+	Type    ActionType
+	Payload interface{}
+
+	// Dismisses specifies if the notification is dismissed when this action is selected.
+	Dismisses bool
+}
+
+// ActionType defines a specific type of action.
+type ActionType string
+
+// Action Types.
+const (
+	ActionTypeNone        = ""             // Report selected ID back to backend.
+	ActionTypeOpenURL     = "open-url"     // Open external URL
+	ActionTypeOpenPage    = "open-page"    // Payload: Page ID
+	ActionTypeOpenSetting = "open-setting" // Payload: See struct definition below.
+	ActionTypeOpenProfile = "open-profile" // Payload: Scoped Profile ID
+	ActionTypeInjectEvent = "inject-event" // Payload: Event ID
+	ActionTypeWebhook     = "call-webhook" // Payload: See struct definition below.
+)
+
+// ActionTypeOpenSettingPayload defines the payload for the OpenSetting Action Type.
+type ActionTypeOpenSettingPayload struct {
+	// Key is the key of the setting.
+	Key string
+	// Profile is the scoped ID of the profile.
+	// Leaving this empty opens the global settings.
+	Profile string
+}
+
+// ActionTypeWebhookPayload defines the payload for the WebhookPayload Action Type.
+type ActionTypeWebhookPayload struct {
+	// HTTP Method to use. Defaults to "GET", or "POST" if a Payload is supplied.
+	Method string
+	// URL to call.
+	// If the URL is relative, prepend the current API endpoint base path.
+	// If the URL is absolute, send request to the Portmaster.
+	URL string
+	// Payload holds arbitrary payload data.
+	Payload interface{}
+	// ResultAction defines what should be done with successfully returned data.
+	// Must one of:
+	// - `ignore`: do nothing (default)
+	// - `display`: the result is a human readable message, display it in a success message.
+	ResultAction string
 }
 
 // Get returns the notification identifed by the given id or nil if it doesn't exist.
@@ -114,28 +168,55 @@ func Get(id string) *Notification {
 	return nil
 }
 
+// Delete deletes the notification with the given id.
+func Delete(id string) {
+	// Delete notification in defer to enable deferred unlocking.
+	var n *Notification
+	var ok bool
+	defer func() {
+		if ok {
+			n.Delete()
+		}
+	}()
+
+	notsLock.Lock()
+	defer notsLock.Unlock()
+	n, ok = nots[id]
+}
+
 // NotifyInfo is a helper method for quickly showing a info
 // notification. The notification is already shown. If id is
 // an empty string a new UUIDv4 will be generated.
-func NotifyInfo(id, msg string, actions ...Action) *Notification {
-	return notify(Info, id, msg, actions...)
+// ShowOnSystem is disabled.
+func NotifyInfo(id, title, msg string, actions ...Action) *Notification {
+	return notify(Info, id, title, msg, false, actions...)
 }
 
 // NotifyWarn is a helper method for quickly showing a warning
 // notification. The notification is already shown. If id is
 // an empty string a new UUIDv4 will be generated.
-func NotifyWarn(id, msg string, actions ...Action) *Notification {
-	return notify(Warning, id, msg, actions...)
+// ShowOnSystem is enabled.
+func NotifyWarn(id, title, msg string, actions ...Action) *Notification {
+	return notify(Warning, id, title, msg, true, actions...)
+}
+
+// NotifyError is a helper method for quickly showing an error
+// notification. The notification is already shown. If id is
+// an empty string a new UUIDv4 will be generated.
+// ShowOnSystem is enabled.
+func NotifyError(id, title, msg string, actions ...Action) *Notification {
+	return notify(Error, id, title, msg, true, actions...)
 }
 
 // NotifyPrompt is a helper method for quickly showing a prompt
 // notification. The notification is already shown. If id is
 // an empty string a new UUIDv4 will be generated.
-func NotifyPrompt(id, msg string, actions ...Action) *Notification {
-	return notify(Prompt, id, msg, actions...)
+// ShowOnSystem is disabled.
+func NotifyPrompt(id, title, msg string, actions ...Action) *Notification {
+	return notify(Prompt, id, title, msg, false, actions...)
 }
 
-func notify(nType Type, id, msg string, actions ...Action) *Notification {
+func notify(nType Type, id, title, msg string, showOnSystem bool, actions ...Action) *Notification {
 	acts := make([]*Action, len(actions))
 	for idx := range actions {
 		a := actions[idx]
@@ -145,8 +226,10 @@ func notify(nType Type, id, msg string, actions ...Action) *Notification {
 	return Notify(&Notification{
 		EventID:          id,
 		Type:             nType,
+		Title:            title,
 		Message:          msg,
 		AvailableActions: acts,
+		ShowOnSystem:     showOnSystem,
 	})
 }
 
@@ -185,15 +268,9 @@ func (n *Notification) save(pushUpdate bool) {
 	n.lock.Lock()
 	defer n.lock.Unlock()
 
-	// Move Title to Message, as that is the required field.
-	if n.Message == "" {
-		n.Message = n.Title
-		n.Title = ""
-	}
-
 	// Check if required data is present.
-	if n.Message == "" {
-		log.Warning("notifications: ignoring notification without Message")
+	if n.Title == "" && n.Message == "" {
+		log.Warning("notifications: ignoring notification without Title or Message")
 		return
 	}
 
@@ -293,9 +370,8 @@ func (n *Notification) Update(expires int64) {
 }
 
 // Delete (prematurely) cancels and deletes a notification.
-func (n *Notification) Delete() error {
+func (n *Notification) Delete() {
 	n.delete(true)
-	return nil
 }
 
 // delete deletes the notification from the internal storage. It locks the
@@ -332,6 +408,8 @@ func (n *Notification) delete(pushUpdate bool) {
 	if pushUpdate {
 		dbController.PushUpdate(n)
 	}
+
+	n.resolveModuleFailure()
 }
 
 // Expired notifies the caller when the notification has expired.
@@ -384,6 +462,7 @@ func (n *Notification) selectAndExecuteAction(id string) {
 
 	if executed {
 		n.State = Executed
+		n.resolveModuleFailure()
 	}
 }