Skip to content
This repository was archived by the owner on Feb 23, 2021. It is now read-only.

Conversation

@valentinewallace
Copy link
Contributor

@valentinewallace valentinewallace commented Sep 12, 2018

Time out payments after 1 minute.

Closes #555

const timeout = setTimeout(() => resolve(false), PAYMENT_TIMEOUT);
stream.on('data', data => {
if (data.payment_error) {
clearTimeout(timeout);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we're clearing the timeout here regardless of the conditional. Can we move it outside the conditional (below line 179) and delete line 184?

Copy link
Contributor

@tanx tanx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@valentinewallace I refactored the code a bit because it would have navigated twice in some cases. Let me know if it makes sense like this. Good to merge from my side.

*/
async payLightning() {
let failed = false;
const timeout = setTimeout(() => {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can I test this block manually? In my local simnet environment I'm always getting an error from the stream data.payment_error and it just shows the notification.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested by manually putting a sleep before the failed check, forcing a timeout. Not the most robust 😬

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@valentinewallace well, that's one way of doing it :D.

valentinewallace and others added 6 commits September 13, 2018 17:20
The timeout needs to be in the sendPayment promise to ensure that we resolve the promise on timeout, otherwise it might resolve/reject later on. This could cause undesirable side effects such as navigating to the success screen or showing an unexpected notification.
@valentinewallace valentinewallace merged commit dafb56c into master Sep 13, 2018
@tanx tanx deleted the payment-timeout branch October 1, 2018 09:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants