fix(lnd): handling hold invoice check errors (#1969)

This adds better error handling for when the test calls to verify lnd
hold invoices are available fail due to connectivity reasons. Previously
any error that occurred at this step would cause us to set lnd's status
to `NoHoldInvoiceSupport` including connection issues. There was also a
bug that caused us to try to set the status to connected even when a
hold invoice status check failed.

This could result in the unusual behavior of status going to
`Disconnected` upon a call failing due to the grpc `UNAVAILABLE` error
status, then being set to `NoHoldInvoiceSupport` and then to
`ConnectionVerified`. Now we only set `NoHoldInvoiceSupport` when the
test calls fail for a reason other than `UNAVAILABLE`, and we only set
the status to `ConnectionVerified` when the hold invoice calls succeed.

Closes #1968.

Author: sangaman

Diff for: lib/lndclient/LndClient.ts

@@ -497,6 +497,12 @@ class LndClient extends SwapClient {
           this.setStatus(ClientStatus.Misconfigured);
         }
 
+        if (this.walletUnlocker) {
+          // WalletUnlocker service is disabled when the main Lightning service is available
+          this.walletUnlocker.close();
+          this.walletUnlocker = undefined;
+        }
+
         this.invoices = new InvoicesClient(this.uri, this.credentials);
         try {
           const randomHash = crypto.randomBytes(32).toString('hex');
@@ -505,16 +511,13 @@ class LndClient extends SwapClient {
           await this.addInvoice({ rHash: randomHash, units: 1 });
           await this.removeInvoice(randomHash);
         } catch (err) {
-          const errStr = typeof(err) === 'string' ? err : JSON.stringify(err);
-
-          this.logger.error(`could not add hold invoice, error: ${errStr}`);
-          this.setStatus(ClientStatus.NoHoldInvoiceSupport);
-        }
-
-        if (this.walletUnlocker) {
-          // WalletUnlocker service is disabled when the main Lightning service is available
-          this.walletUnlocker.close();
-          this.walletUnlocker = undefined;
+          if (err.code !== grpc.status.UNAVAILABLE) {
+            // mark the client as not having hold invoice support if the invoice calls failed due to
+            // reasons other than generic grpc connectivity errors
+            this.logger.error('could not add hold invoice', err);
+            this.setStatus(ClientStatus.NoHoldInvoiceSupport);
+          }
+          throw err; // we don't want to proceed with marking the client as connected, regardless of the error
         }
 
         await this.setConnected(newPubKey, newUris);

Diff for: lib/swaps/SwapClient.ts

@@ -206,7 +206,7 @@ abstract class SwapClient extends EventEmitter {
       case ClientStatus.WaitingUnlock:
       case ClientStatus.OutOfSync:
       case ClientStatus.NoHoldInvoiceSupport:
-        // these statuses can only be set on an operational, initalized client
+        // these statuses can only be set on an operational, initialized client
         validStatusTransition = this.isOperational();
         break;
       case ClientStatus.NotInitialized:
@@ -349,7 +349,7 @@ abstract class SwapClient extends EventEmitter {
    * Returns `true` if the client is enabled and configured properly.
    */
   public isOperational(): boolean {
-    return !this.isDisabled() && !this.isMisconfigured() && !this.isNotInitialized() && !this.hasNoInvoiceSupport();
+    return !this.isDisabled() && !this.isMisconfigured() && !this.isNotInitialized();
   }
   public isDisconnected(): boolean {
     return this.status === ClientStatus.Disconnected;