From da761575541809a3a91fad3e469aa3079f435f65 Mon Sep 17 00:00:00 2001
From: "kali kaneko (leap communications)" <kali@leap.se>
Date: Thu, 11 Mar 2021 04:37:51 +0100
Subject: [PATCH] [bug] fail gracefully on connection initialization errors

There's some corner cases that were very badly captured. On the first
place, it's confusing to quit on connection errors.

Secondly, a side-effect of aborting the initialization of the bitmask
object was a semi-random segfault when trying to access the object.

Here I pass any connection errors to the gui, but leave to the user to
quit the app. This probably will need more work when we want to
terminate the app on unrecoverable errors (no polkit, etc...), but for
now it makes the systray much more usable.

- Resolves: #465
---
 gui/qml/FailDialog.qml |  6 +++++-
 pkg/backend/actions.go |  6 +++---
 pkg/backend/api.go     |  6 ++++++
 pkg/backend/init.go    | 11 +++++------
 pkg/bitmask/init.go    |  4 +++-
 pkg/vpn/openvpn.go     |  3 ++-
 6 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/gui/qml/FailDialog.qml b/gui/qml/FailDialog.qml
index a78690e5..d151feb3 100644
--- a/gui/qml/FailDialog.qml
+++ b/gui/qml/FailDialog.qml
@@ -16,7 +16,11 @@ MessageDialog {
         if (ctx.loginDialog == 'true') {
             login.visible = true
         } else {
-            backend.quit()
+            // FIXME - we probably want to distinguish 
+            // fatal from recoverable errors. For the time being
+            // we can avoid quitting so that people can try reconnects if it's
+            // a network problem, it's confusing to quit the app.
+            // backend.quit()
         }
     }
 }
diff --git a/pkg/backend/actions.go b/pkg/backend/actions.go
index 725a550f..e45b026e 100644
--- a/pkg/backend/actions.go
+++ b/pkg/backend/actions.go
@@ -2,14 +2,14 @@ package backend
 
 import (
 	"log"
-	"os"
 )
 
 func startVPN() {
+	setError("")
 	err := ctx.bm.StartVPN(ctx.Provider)
 	if err != nil {
-		log.Println(err)
-		os.Exit(1)
+		log.Println("ERROR: ", err)
+		setError(err.Error())
 	}
 }
 
diff --git a/pkg/backend/api.go b/pkg/backend/api.go
index a799b0eb..293dd9e6 100644
--- a/pkg/backend/api.go
+++ b/pkg/backend/api.go
@@ -38,6 +38,12 @@ func Login(username, password string) {
 	go ctx.updateStatus()
 }
 
+func setError(err string) {
+	ctx.Errors = err
+	go setStatus(off)
+	go ctx.updateStatus()
+}
+
 func SwitchOn() {
 	go setStatus(starting)
 	go startVPN()
diff --git a/pkg/backend/init.go b/pkg/backend/init.go
index f76ab49b..7d7a5342 100644
--- a/pkg/backend/init.go
+++ b/pkg/backend/init.go
@@ -47,7 +47,7 @@ func checkErrors(errCh chan string) {
 
 func initializeBitmask(errCh chan string, opts *InitOpts) {
 	if ctx == nil {
-		log.Println("bug: cannot initialize bitmask, ctx is nil!")
+		log.Println("BUG: cannot initialize bitmask, ctx is nil!")
 		os.Exit(1)
 	}
 	bitmask.InitializeLogger()
@@ -63,7 +63,7 @@ func initializeBitmask(errCh chan string, opts *InitOpts) {
 
 	b, err := bitmask.InitializeBitmask(ctx.cfg)
 	if err != nil {
-		log.Println("error: cannot initialize bitmask")
+		log.Println("ERROR: cannot initialize bitmask")
 		errCh <- err.Error()
 		return
 	}
@@ -72,19 +72,18 @@ func initializeBitmask(errCh chan string, opts *InitOpts) {
 	helpers, privilege, err := b.VPNCheck()
 
 	if err != nil {
-		log.Println("error doing vpn check")
+		log.Println("ERROR: vpn check")
 		errCh <- err.Error()
 	}
 
 	if helpers == false {
-		log.Println("no helpers")
+		log.Println("ERROR: no helpers")
 		errCh <- "nohelpers"
 	}
 	if privilege == false {
-		log.Println("no polkit")
+		log.Println("ERROR: no polkit")
 		errCh <- "nopolkit"
 	}
-
 	ctx.bm = b
 }
 
diff --git a/pkg/bitmask/init.go b/pkg/bitmask/init.go
index 02ee544d..b6b41b41 100644
--- a/pkg/bitmask/init.go
+++ b/pkg/bitmask/init.go
@@ -97,8 +97,10 @@ func InitializeBitmask(conf *config.Config) (Bitmask, error) {
 	if !conf.SkipLaunch {
 		err := maybeStartVPN(b, conf)
 		if err != nil {
+			// we don't want this error to avoid initialization of
+			// the bitmask object. If we cannot autostart it's not
+			// so terrible.
 			log.Println("Error starting VPN: ", err)
-			return nil, err
 		}
 	}
 	return b, nil
diff --git a/pkg/vpn/openvpn.go b/pkg/vpn/openvpn.go
index c0ec2e17..d5f9dc2f 100644
--- a/pkg/vpn/openvpn.go
+++ b/pkg/vpn/openvpn.go
@@ -47,7 +47,8 @@ func (b *Bitmask) StartVPN(provider string) error {
 	if !b.CanStartVPN() {
 		return errors.New("BUG: cannot start vpn")
 	}
-	return b.startOpenVPN(proxy)
+	err := b.startOpenVPN(proxy)
+	return err
 }
 
 func (b *Bitmask) CanStartVPN() bool {
-- 
GitLab