-
Notifications
You must be signed in to change notification settings - Fork 43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: show LND info while it is out of sync #1294
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me know what you think of these suggestions, thanks.
lib/lndclient/LndClient.ts
Outdated
@@ -238,7 +236,10 @@ class LndClient extends SwapClient { | |||
uris = getInfoResponse.getUrisList(); | |||
version = getInfoResponse.getVersion(); | |||
alias = getInfoResponse.getAlias(); | |||
if (channels.active <= 0) { | |||
|
|||
if (!this.isConnected()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would make sense to add a new isOutOfSync()
method for SwapClient
and check that here, then throw a new error OUT_OF_SYNC
instead of UNAVAILABLE
. That way callers can tell whether lnd is actually unavailable/offline or if it's up but simply out of sync and not ready.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because of this.status
in errors.UNAVAILABLE(this.currency, this.status).message
the error message already shows that LND is out of sync.
I agree on isOutOfSync()
but don't think that a new error is necessary because of the reason stated above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I overlooked that.
8529f34
to
485aa8f
Compare
Closes #1276