From 1dbc9a136612725d63f21e55beecb5988e3ea437 Mon Sep 17 00:00:00 2001
From: James Houlahan <james.houlahan@proton.ch>
Date: Tue, 1 Nov 2022 13:16:32 +0100
Subject: [PATCH] GODT-2010: Add better logging for app focus feature

---
 internal/app/singleinstance_unix.go |  4 ++
 internal/focus/client.go            | 90 ++++++++++++++---------------
 2 files changed, 49 insertions(+), 45 deletions(-)

diff --git a/internal/app/singleinstance_unix.go b/internal/app/singleinstance_unix.go
index 94976300..f7c98130 100644
--- a/internal/app/singleinstance_unix.go
+++ b/internal/app/singleinstance_unix.go
@@ -31,6 +31,7 @@ import (
 	"github.com/Masterminds/semver/v3"
 	"github.com/ProtonMail/proton-bridge/v2/internal/focus"
 	"github.com/allan-simon/go-singleinstance"
+	"github.com/sirupsen/logrus"
 	"golang.org/x/sys/unix"
 )
 
@@ -42,9 +43,12 @@ import (
 // it will kill old and continue with this new bridge (i.e. no error returned).
 func checkSingleInstance(lockFilePath string, curVersion *semver.Version) (*os.File, error) {
 	if lock, err := singleinstance.CreateLockFile(lockFilePath); err == nil {
+		logrus.WithField("path", lockFilePath).Debug("Created lock file; no other instance is running")
 		return lock, nil
 	}
 
+	logrus.Debug("Failed to create lock file; another instance is running")
+
 	// We couldn't create the lock file, so another instance is probably running.
 	// Check if it's an older version of the app.
 	lastVersion, ok := focus.TryVersion()
diff --git a/internal/focus/client.go b/internal/focus/client.go
index 25cd04db..a90a79ca 100644
--- a/internal/focus/client.go
+++ b/internal/focus/client.go
@@ -21,7 +21,6 @@ import (
 	"context"
 	"fmt"
 	"net"
-	"time"
 
 	"github.com/Masterminds/semver/v3"
 	"github.com/ProtonMail/proton-bridge/v2/internal/focus/proto"
@@ -34,65 +33,66 @@ import (
 // TryRaise tries to raise the application by dialing the focus service.
 // It returns true if the service is running and the application was told to raise.
 func TryRaise() bool {
-	ctx, cancel := context.WithTimeout(context.Background(), time.Second)
-	defer cancel()
+	var raised bool
 
-	cc, err := grpc.DialContext(
-		ctx,
-		net.JoinHostPort(Host, fmt.Sprint(Port)),
-		grpc.WithTransportCredentials(insecure.NewCredentials()),
-	)
-	if err != nil {
-		return false
-	}
-
-	defer func() {
-		if err := cc.Close(); err != nil {
-			logrus.WithError(err).Warn("Failed to close focus connection")
+	if err := withClientConn(context.Background(), func(ctx context.Context, client proto.FocusClient) error {
+		if _, err := client.Raise(ctx, &emptypb.Empty{}); err != nil {
+			return fmt.Errorf("failed to call client.Raise: %w", err)
 		}
-	}()
 
-	if _, err := proto.NewFocusClient(cc).Raise(ctx, &emptypb.Empty{}); err != nil {
+		raised = true
+
+		return nil
+	}); err != nil {
+		logrus.WithError(err).Debug("Failed to raise application")
 		return false
 	}
 
-	if err := cc.Close(); err != nil {
-		return false
-	}
-
-	return true
+	return raised
 }
 
-// TryVersion tries to raise the application by dialing the focus service.
-// It returns true if the service is running and the application was told to raise.
+// TryVersion tries to determine the version of the running application instance.
+// It returns the version and true if the version could be determined.
 func TryVersion() (*semver.Version, bool) {
-	ctx, cancel := context.WithTimeout(context.Background(), time.Second)
-	defer cancel()
+	var version *semver.Version
 
-	cc, err := grpc.DialContext(
-		ctx,
-		net.JoinHostPort(Host, fmt.Sprint(Port)),
-		grpc.WithTransportCredentials(insecure.NewCredentials()),
-	)
-	if err != nil {
-		return nil, false
-	}
-
-	defer func() {
-		if err := cc.Close(); err != nil {
-			logrus.WithError(err).Warn("Failed to close focus connection")
+	if err := withClientConn(context.Background(), func(ctx context.Context, client proto.FocusClient) error {
+		raw, err := client.Version(ctx, &emptypb.Empty{})
+		if err != nil {
+			return fmt.Errorf("failed to call client.Version: %w", err)
 		}
-	}()
 
-	raw, err := proto.NewFocusClient(cc).Version(ctx, &emptypb.Empty{})
-	if err != nil {
-		return nil, false
-	}
+		parsed, err := semver.NewVersion(raw.GetVersion())
+		if err != nil {
+			return fmt.Errorf("failed to parse version: %w", err)
+		}
 
-	version, err := semver.NewVersion(raw.GetVersion())
-	if err != nil {
+		version = parsed
+
+		return nil
+	}); err != nil {
+		logrus.WithError(err).Debug("Failed to determine version of running instance")
 		return nil, false
 	}
 
 	return version, true
 }
+
+func withClientConn(ctx context.Context, fn func(context.Context, proto.FocusClient) error) error {
+	cc, err := grpc.DialContext(
+		ctx,
+		net.JoinHostPort(Host, fmt.Sprint(Port)),
+		grpc.WithTransportCredentials(insecure.NewCredentials()),
+	)
+	if err != nil {
+		return err
+	}
+
+	defer func() {
+		if err := cc.Close(); err != nil {
+			logrus.WithError(err).Warn("Failed to close focus connection")
+		}
+	}()
+
+	return fn(ctx, proto.NewFocusClient(cc))
+}