fix: retry auto-unlock StartApp on failure instead of silently staying locked

api/api.go

@@ -18,7 +18,6 @@ import (
 	"slices"
 	"strconv"
 	"strings"
-	"sync"
 	"sync/atomic"
 	"time"
 
@@ -837,22 +836,11 @@ func (api *api) SetAutoUnlockPassword(unlockPassword string) error {
 }
 
 func (api *api) Stop() error {
-	if !startMutex.TryLock() {
-		// do not allow to stop twice in case this is somehow called twice
-		return errors.New("app is busy")
-	}
-	defer startMutex.Unlock()
-
 	logger.Logger.Info("Running Stop command")
-	if api.svc.GetLNClient() == nil {
-		return ErrLNClientNotStarted
-	}
 
 	// stop the lnclient, nostr relay etc.
 	// The user will be forced to re-enter their unlock password to restart the node
-	api.svc.StopApp()
-
-	return nil
+	return api.svc.StopApp()
 }
 
 func (api *api) GetNodeConnectionInfo(ctx context.Context) (*NodeConnectionInfo, error) {
@@ -1722,33 +1710,24 @@ func (api *api) SetNextBackupReminder(backupReminderRequest *BackupReminderReque
 	return nil
 }
 
-var startMutex sync.Mutex
-
 func (api *api) Start(startRequest *StartRequest) {
 	api.startupError = nil
-	err := api.startInternal(startRequest)
+	err := api.svc.StartApp(startRequest.UnlockPassword)
 	if err != nil {
 		logger.Logger.WithError(err).Error("Failed to start node")
 		api.startupError = err
 		api.startupErrorTime = time.Now()
 	}
 }
 
-func (api *api) startInternal(startRequest *StartRequest) (err error) {
-	if !startMutex.TryLock() {
-		// do not allow to start twice in case this is somehow called twice
-		return errors.New("app is busy")
-	}
-	defer startMutex.Unlock()
-	return api.svc.StartApp(startRequest.UnlockPassword)
+func (api *api) Setup(ctx context.Context, setupRequest *SetupRequest) error {
+	// hold the start/stop lock so setup cannot run concurrently with a start
+	return api.svc.WithStartLock(func() error {
+		return api.setupInternal(ctx, setupRequest)
+	})
 }
 
-func (api *api) Setup(ctx context.Context, setupRequest *SetupRequest) error {
-	if !startMutex.TryLock() {
-		// do not allow to start twice in case this is somehow called twice
-		return errors.New("app is busy")
-	}
-	defer startMutex.Unlock()
+func (api *api) setupInternal(ctx context.Context, setupRequest *SetupRequest) error {
 	info, err := api.GetInfo(ctx)
 	if err != nil {
 		logger.Logger.WithError(err).Error("Failed to get info")

api/backup.go

@@ -105,7 +105,10 @@ func (api *api) CreateBackup(unlockPassword string, w io.Writer) error {
 		return fmt.Errorf("failed to reset router: %w", err)
 	}
 	// Stop the app to ensure no new requests are processed.
-	api.svc.StopApp()
+	err = api.svc.StopApp()
+	if err != nil {
+		return fmt.Errorf("failed to stop app: %w", err)
+	}
 
 	// Remove the OAuth access token from the DB to ensure the user
 	// has to re-auth with the correct OAuth client when they restore the backup

api/backup_test.go

@@ -61,7 +61,7 @@ func TestCreateBackup(t *testing.T) {
 
 	svc := mocks.NewMockService(t)
 	svc.On("GetLNClient").Return(lnClient)
-	svc.On("StopApp").Return()
+	svc.On("StopApp").Return(nil)
 
 	albyOAuthSvc := mocks.NewMockAlbyOAuthService(t)
 	albyOAuthSvc.On("RemoveOAuthAccessToken").Return(nil)

lnclient/lnd/lnd.go

@@ -60,28 +60,11 @@ func NewLNDService(ctx context.Context, eventPublisher events.EventPublisher, ln
 		return nil, err
 	}
 
-	var nodeInfo *lnclient.NodeInfo
-	maxRetries := 60
-	for i := range maxRetries {
-		nodeInfo, err = fetchNodeInfo(ctx, lndClient)
-		if err == nil {
-			break
-		}
-		logger.Logger.WithFields(logrus.Fields{
-			"iteration": i,
-		}).WithError(err).Error("Failed to connect to LND, retrying in 10s")
-
-		select {
-		case <-time.After(10 * time.Second):
-		case <-ctx.Done():
-			logger.Logger.WithError(ctx.Err()).Error("Context cancelled during LND connection retries")
-			return nil, ctx.Err()
-		}
-	}
-
+	// confirm LND is running by fetching the node info
+	nodeInfo, err := fetchNodeInfo(ctx, lndClient)
 	if err != nil {
-		logger.Logger.WithError(err).Error("Failed to connect to LND on final attempt, not attempting further retries")
-		return nil, err
+		logger.Logger.WithError(err).Error("Failed to connect to LND")
+		return nil, fmt.Errorf("connect to LND: %w", err)
 	}
 
 	lndCtx, cancel := context.WithCancel(ctx)

service/models.go

@@ -1,6 +1,8 @@
 package service
 
 import (
+	"errors"
+
 	"gorm.io/gorm"
 
 	"github.com/getAlby/hub/alby"
@@ -12,14 +14,30 @@ import (
 	"github.com/getAlby/hub/transactions"
 )
 
+var (
+	// ErrAppBusy is returned when another start or stop operation is already in progress.
+	ErrAppBusy = errors.New("app is busy")
+	// ErrAlreadyStarted is returned when the app is already unlocked and running.
+	ErrAlreadyStarted = errors.New("app already started")
+	// ErrAppNotStarted is returned when trying to stop an app that is not running.
+	ErrAppNotStarted = errors.New("app not started")
+	// ErrInvalidPassword is returned when the provided unlock password is incorrect.
+	ErrInvalidPassword = errors.New("invalid password")
+	// ErrIncompleteWalletData is returned when the unlock password check is missing from the database.
+	ErrIncompleteWalletData = errors.New("your wallet data is incomplete and cannot be unlocked. Please restore from a backup")
+)
+
 type RelayStatus struct {
 	Url    string
 	Online bool
 }
 
 type Service interface {
 	StartApp(encryptionKey string) error
-	StopApp()
+	StopApp() error
+	// WithStartLock runs fn while holding the start/stop lock,
+	// returning ErrAppBusy if a start or stop is already in progress.
+	WithStartLock(fn func() error) error
 	Shutdown()
 
 	// TODO: remove getters (currently used by http / wails services)

service/service.go

@@ -45,6 +45,7 @@ type service struct {
 	wg                   *sync.WaitGroup
 	nip47Service         nip47.Nip47Service
 	appCancelFn          context.CancelFunc
+	startMutex           sync.Mutex
 	keys                 keys.Keys
 	relayStatuses        []RelayStatus
 	startupState         string
@@ -157,7 +158,10 @@ func NewService(ctx context.Context) (*service, error) {
 	if autoUnlockPassword != "" {
 		nodeLastStartTime, _ := cfg.Get("NodeLastStartTime", "")
 		if nodeLastStartTime != "" {
-			svc.StartApp(autoUnlockPassword)
+			// do not block startup of the web UI: if the start attempt fails
+			// (e.g. the hub boots before the internet connection is restored),
+			// keep retrying in the background instead of staying locked
+			go svc.startAppWithRetries(autoUnlockPassword)
 		}
 	}
 
@@ -226,7 +230,13 @@ func finishRestoreNode(workDir string) error {
 }
 
 func (svc *service) Shutdown() {
-	svc.StopApp()
+	// unlike StopApp, shutdown must never bail out: block until any
+	// in-progress start or stop has finished (the service context is
+	// already cancelled at this point, so an in-progress start aborts),
+	// then stop the app before tearing down the event publisher and DB
+	svc.startMutex.Lock()
+	defer svc.startMutex.Unlock()
+	svc.stopAppInternal()
 	svc.eventPublisher.PublishSync(&events.Event{
 		Event: "nwc_stopped",
 	})

service/start.go

@@ -256,7 +256,56 @@ func (svc *service) watchSubscription(ctx context.Context, pool *nostr.SimplePoo
 	}
 }
 
+// startAppWithRetries runs StartApp with capped exponential backoff until it
+// succeeds or fails permanently. It is used for auto-unlock on startup: a hub
+// that boots before its internet connection is restored (e.g. after a power
+// cut) must not silently stay locked because the first start attempt failed.
+func (svc *service) startAppWithRetries(encryptionKey string) {
+	backoff := 10 * time.Second
+	const maxBackoff = 5 * time.Minute
+
+	for {
+		err := svc.StartApp(encryptionKey)
+		if err == nil {
+			return
+		}
+		if errors.Is(err, ErrAlreadyStarted) {
+			logger.Logger.Info("App was started manually, stopping auto-unlock retries")
+			return
+		}
+		if errors.Is(err, ErrInvalidPassword) || errors.Is(err, ErrIncompleteWalletData) {
+			logger.Logger.WithError(err).Error("Auto-unlock failed permanently, not retrying")
+			return
+		}
+
+		logger.Logger.WithError(err).WithField("retry_in", backoff.String()).Error("Auto-unlock failed, retrying")
+		select {
+		case <-svc.ctx.Done():
+			return
+		case <-time.After(backoff):
+		}
+		backoff = min(backoff*2, maxBackoff)
+	}
+}
+
+// WithStartLock runs fn while holding the start/stop lock, returning
+// ErrAppBusy if a start, stop or setup operation is already in progress.
+func (svc *service) WithStartLock(fn func() error) error {
+	if !svc.startMutex.TryLock() {
+		return ErrAppBusy
+	}
+	defer svc.startMutex.Unlock()
+	return fn()
+}
+
 func (svc *service) StartApp(encryptionKey string) error {
+	// do not allow to start twice in case this is somehow called twice
+	return svc.WithStartLock(func() error {
+		return svc.startAppInternal(encryptionKey)
+	})
+}
+
+func (svc *service) startAppInternal(encryptionKey string) error {
 	defer func() {
 		svc.startupState = ""
 	}()
@@ -271,7 +320,7 @@ func (svc *service) StartApp(encryptionKey string) error {
 	}
 
 	if svc.lnClient != nil {
-		return errors.New("app already started")
+		return ErrAlreadyStarted
 	}
 	unlockPasswordCheckSet, err := svc.cfg.IsUnlockPasswordCheckSet()
 	if err != nil {
@@ -280,11 +329,11 @@ func (svc *service) StartApp(encryptionKey string) error {
 	}
 	if !unlockPasswordCheckSet {
 		logger.Logger.Error("Unlock password check is missing from the database")
-		return errors.New("your wallet data is incomplete and cannot be unlocked. Please restore from a backup")
+		return ErrIncompleteWalletData
 	}
 	if !svc.cfg.CheckUnlockPassword(encryptionKey) {
 		logger.Logger.Errorf("Invalid password")
-		return errors.New("invalid password")
+		return ErrInvalidPassword
 	}
 
 	err = svc.cfg.LoadJWTSecret(encryptionKey)

service/stop.go

@@ -7,7 +7,21 @@ import (
 	"github.com/getAlby/hub/logger"
 )
 
-func (svc *service) StopApp() {
+func (svc *service) StopApp() error {
+	// do not allow stopping while a start or stop is already in progress
+	return svc.WithStartLock(func() error {
+		// check under the lock so an in-progress start reports busy
+		// rather than not started
+		if svc.lnClient == nil {
+			return ErrAppNotStarted
+		}
+		svc.stopAppInternal()
+		return nil
+	})
+}
+
+// stopAppInternal must be called while holding the start lock
+func (svc *service) stopAppInternal() {
 	if svc.appCancelFn != nil {
 		logger.Logger.Info("Stopping app...")
 		svc.appCancelFn()