From 6ffccc9ee694339c500258446fabaa3d1bd3f584 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Sun, 9 Sep 2018 17:52:06 -0700 Subject: [PATCH 1/6] Rename NoRouteFound view to PaymentFailed. --- src/view/{no-route.js => payment-failed.js} | 10 +++++----- stories/screen-story.js | 6 ++++-- 2 files changed, 9 insertions(+), 7 deletions(-) rename src/view/{no-route.js => payment-failed.js} (85%) diff --git a/src/view/no-route.js b/src/view/payment-failed.js similarity index 85% rename from src/view/no-route.js rename to src/view/payment-failed.js index af2de0a61..a85781034 100644 --- a/src/view/no-route.js +++ b/src/view/payment-failed.js @@ -28,14 +28,14 @@ const styles = StyleSheet.create({ }, }); -const NoRouteView = ({ channel, payment }) => ( +const PaymentFailedView = ({ channel, payment }) => ( - No route found + Payment Failed - {"You'll need to manually create a channel"} + {'You may need to manually create a channel.'} channel.initCreate()}> @@ -48,9 +48,9 @@ const NoRouteView = ({ channel, payment }) => ( ); -NoRouteView.propTypes = { +PaymentFailedView.propTypes = { channel: PropTypes.object.isRequired, payment: PropTypes.object.isRequired, }; -export default observer(NoRouteView); +export default observer(PaymentFailedView); diff --git a/stories/screen-story.js b/stories/screen-story.js index 5da47ede7..6de7b8b05 100644 --- a/stories/screen-story.js +++ b/stories/screen-story.js @@ -38,7 +38,7 @@ import PayLightningDone from '../src/view/pay-lightning-done'; import PayBitcoin from '../src/view/pay-bitcoin'; import PayBitcoinConfirm from '../src/view/pay-bitcoin-confirm'; import PayBitcoinDone from '../src/view/pay-bitcoin-done'; -import NoRoute from '../src/view/no-route'; +import PaymentFailed from '../src/view/payment-failed'; import Loader from '../src/view/loader'; import LoaderSyncing from '../src/view/loader-syncing'; import SelectSeed from '../src/view/select-seed'; @@ -149,7 +149,9 @@ storiesOf('Screens', module) .add('Pay Lightning Done', () => ( )) - .add('No Route Found', () => ) + .add('Payment Failed', () => ( + + )) .add('Pay Bitcoin', () => ( )) From 6755c4877582b2fdb9542252a0730786775bf552 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Sun, 9 Sep 2018 17:54:09 -0700 Subject: [PATCH 2/6] Hook up PaymentFailed screen in main/nav. --- src/action/nav.js | 4 ++++ src/view/main.js | 4 ++++ test/unit/action/nav.spec.js | 7 +++++++ 3 files changed, 15 insertions(+) diff --git a/src/action/nav.js b/src/action/nav.js index d4560c79c..cba7268ad 100644 --- a/src/action/nav.js +++ b/src/action/nav.js @@ -73,6 +73,10 @@ class NavAction { this._store.route = 'PayLightningDone'; } + goPaymentFailed() { + this._store.route = 'PaymentFailed'; + } + goPayBitcoin() { this._store.route = 'PayBitcoin'; } diff --git a/src/view/main.js b/src/view/main.js index 3f79c28e6..64e1ad09c 100644 --- a/src/view/main.js +++ b/src/view/main.js @@ -19,6 +19,7 @@ import Home from './home'; import Payment from './payment'; import PayLightningConfirm from './pay-lightning-confirm'; import PayLightningDone from './pay-lightning-done'; +import PaymentFailed from './payment-failed'; import PayBitcoin from './pay-bitcoin'; import PayBitcoinConfirm from './pay-bitcoin-confirm'; import PayBitcoinDone from './pay-bitcoin-done'; @@ -111,6 +112,9 @@ class MainView extends Component { {route === 'PayLightningDone' && ( )} + {route === 'PaymentFailed' && ( + + )} {route === 'PayBitcoin' && ( )} diff --git a/test/unit/action/nav.spec.js b/test/unit/action/nav.spec.js index 86e2b1c29..54163d50f 100644 --- a/test/unit/action/nav.spec.js +++ b/test/unit/action/nav.spec.js @@ -130,6 +130,13 @@ describe('Action Nav Unit Tests', () => { }); }); + describe('goPaymentFailed()', () => { + it('should set correct route', () => { + nav.goPaymentFailed(); + expect(store.route, 'to equal', 'PaymentFailed'); + }); + }); + describe('goPayBitcoin()', () => { it('should set correct route', () => { nav.goPayBitcoin(); From 5f142ceeaf4216886f94d0d6005fbab5b9cb6c65 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Sun, 9 Sep 2018 17:56:11 -0700 Subject: [PATCH 3/6] Add timeout to payLightning so we go to PaymentFailed on timeout. 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. --- src/action/payment.js | 17 +++++++++++++---- src/config.js | 1 + test/unit/action/payment.spec.js | 7 +++++++ 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/action/payment.js b/src/action/payment.js index 0ac8b27bd..125570b29 100644 --- a/src/action/payment.js +++ b/src/action/payment.js @@ -3,7 +3,7 @@ * call the corresponding GRPC apis for payment management. */ -import { PREFIX_URI } from '../config'; +import { PREFIX_URI, PAYMENT_TIMEOUT } from '../config'; import { toSatoshis, toAmount, @@ -174,17 +174,26 @@ class PaymentAction { this._nav.goWait(); const invoice = this._store.payment.address.replace(PREFIX_URI, ''); const stream = this._grpc.sendStreamCommand('sendPayment'); - await new Promise((resolve, reject) => { + const success = await new Promise((resolve, reject) => { + const timeout = setTimeout(() => resolve(false), PAYMENT_TIMEOUT); stream.on('data', data => { if (data.payment_error) { + clearTimeout(timeout); reject(new Error(`Lightning payment error: ${data.payment_error}`)); } else { - resolve(); + clearTimeout(timeout); + resolve(true); } }); - stream.on('error', reject); + stream.on('error', () => { + clearTimeout(timeout); + reject(); + }); stream.write(JSON.stringify({ payment_request: invoice }), 'utf8'); }); + if (!success) { + this._nav.goPaymentFailed(); + } this._nav.goPayLightningDone(); } catch (err) { this._nav.goPayLightningConfirm(); diff --git a/src/config.js b/src/config.js index 7a9f32b06..c5cf44238 100644 --- a/src/config.js +++ b/src/config.js @@ -6,6 +6,7 @@ module.exports.RETRY_DELAY = 1000; module.exports.LND_INIT_DELAY = 5000; module.exports.NOTIFICATION_DELAY = 5000; module.exports.RATE_DELAY = 15 * 60 * 1000; +module.exports.PAYMENT_TIMEOUT = 60 * 1000; module.exports.LND_PORT = 10006; module.exports.LND_PEER_PORT = 10016; diff --git a/test/unit/action/payment.spec.js b/test/unit/action/payment.spec.js index 3631b8fd1..0c13b08b5 100644 --- a/test/unit/action/payment.spec.js +++ b/test/unit/action/payment.spec.js @@ -22,6 +22,7 @@ describe('Action Payments Unit Tests', () => { store = new Store(); store.settings.displayFiat = false; require('../../../src/config').RETRY_DELAY = 1; + require('../../../src/config').PAYMENT_TIMEOUT = 500; grpc = sinon.createStubInstance(GrpcAction); notification = sinon.createStubInstance(NotificationAction); nav = sinon.createStubInstance(NavAction); @@ -240,5 +241,11 @@ describe('Action Payments Unit Tests', () => { expect(nav.goPayLightningConfirm, 'was called once'); expect(notification.display, 'was called once'); }); + + it('should go to error page on timeout', async () => { + await payment.payLightning({ invoice: 'some-invoice' }); + expect(nav.goPaymentFailed, 'was called once'); + expect(nav.goPayLightningConfirm, 'was not called'); + }); }); }); From 33c37e4054db498198d00d259f49d1e70888e402 Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Thu, 13 Sep 2018 15:16:59 +0200 Subject: [PATCH 4/6] Refactor timeout logic --- src/action/payment.js | 18 ++++++------------ test/unit/action/payment.spec.js | 7 ++++--- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/src/action/payment.js b/src/action/payment.js index 125570b29..6811953b9 100644 --- a/src/action/payment.js +++ b/src/action/payment.js @@ -170,34 +170,28 @@ class PaymentAction { * @return {Promise} */ async payLightning() { + const to = setTimeout(() => this._nav.goPaymentFailed(), PAYMENT_TIMEOUT); try { this._nav.goWait(); const invoice = this._store.payment.address.replace(PREFIX_URI, ''); const stream = this._grpc.sendStreamCommand('sendPayment'); - const success = await new Promise((resolve, reject) => { - const timeout = setTimeout(() => resolve(false), PAYMENT_TIMEOUT); + await new Promise((resolve, reject) => { stream.on('data', data => { if (data.payment_error) { - clearTimeout(timeout); reject(new Error(`Lightning payment error: ${data.payment_error}`)); } else { - clearTimeout(timeout); - resolve(true); + resolve(); } }); - stream.on('error', () => { - clearTimeout(timeout); - reject(); - }); + stream.on('error', reject); stream.write(JSON.stringify({ payment_request: invoice }), 'utf8'); }); - if (!success) { - this._nav.goPaymentFailed(); - } this._nav.goPayLightningDone(); } catch (err) { this._nav.goPayLightningConfirm(); this._notification.display({ msg: 'Lightning payment failed!', err }); + } finally { + clearTimeout(to); } } } diff --git a/test/unit/action/payment.spec.js b/test/unit/action/payment.spec.js index 0c13b08b5..815ddc6a2 100644 --- a/test/unit/action/payment.spec.js +++ b/test/unit/action/payment.spec.js @@ -22,7 +22,7 @@ describe('Action Payments Unit Tests', () => { store = new Store(); store.settings.displayFiat = false; require('../../../src/config').RETRY_DELAY = 1; - require('../../../src/config').PAYMENT_TIMEOUT = 500; + require('../../../src/config').PAYMENT_TIMEOUT = 10; grpc = sinon.createStubInstance(GrpcAction); notification = sinon.createStubInstance(NotificationAction); nav = sinon.createStubInstance(NavAction); @@ -243,9 +243,10 @@ describe('Action Payments Unit Tests', () => { }); it('should go to error page on timeout', async () => { - await payment.payLightning({ invoice: 'some-invoice' }); + payment.payLightning({ invoice: 'some-invoice' }); + await nap(100); expect(nav.goPaymentFailed, 'was called once'); - expect(nav.goPayLightningConfirm, 'was not called'); + expect(nav.goPayLightningDone, 'was not called'); }); }); }); From 13db55a9a9747e07ef717382e1a291b137059936 Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Thu, 13 Sep 2018 15:23:14 +0200 Subject: [PATCH 5/6] Do not navigate to success/fail screen after timeout --- src/action/payment.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/action/payment.js b/src/action/payment.js index 6811953b9..9a70014f1 100644 --- a/src/action/payment.js +++ b/src/action/payment.js @@ -170,7 +170,11 @@ class PaymentAction { * @return {Promise} */ async payLightning() { - const to = setTimeout(() => this._nav.goPaymentFailed(), PAYMENT_TIMEOUT); + let failed = false; + const timeout = setTimeout(() => { + failed = true; + this._nav.goPaymentFailed(); + }, PAYMENT_TIMEOUT); try { this._nav.goWait(); const invoice = this._store.payment.address.replace(PREFIX_URI, ''); @@ -186,12 +190,14 @@ class PaymentAction { stream.on('error', reject); stream.write(JSON.stringify({ payment_request: invoice }), 'utf8'); }); + if (failed) return; this._nav.goPayLightningDone(); } catch (err) { + if (failed) return; this._nav.goPayLightningConfirm(); this._notification.display({ msg: 'Lightning payment failed!', err }); } finally { - clearTimeout(to); + clearTimeout(timeout); } } } From 133c753c73b25f72d26ca475f7ec10a1cf3ae6ba Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Thu, 13 Sep 2018 15:41:32 +0200 Subject: [PATCH 6/6] Go back to lightning confirm screen when pressing try again --- src/view/main.js | 2 +- src/view/payment-failed.js | 9 ++++++--- stories/screen-story.js | 4 +--- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/view/main.js b/src/view/main.js index 64e1ad09c..00d41ca0a 100644 --- a/src/view/main.js +++ b/src/view/main.js @@ -113,7 +113,7 @@ class MainView extends Component { )} {route === 'PaymentFailed' && ( - + )} {route === 'PayBitcoin' && ( diff --git a/src/view/payment-failed.js b/src/view/payment-failed.js index a85781034..6d0b4f8c9 100644 --- a/src/view/payment-failed.js +++ b/src/view/payment-failed.js @@ -28,7 +28,7 @@ const styles = StyleSheet.create({ }, }); -const PaymentFailedView = ({ channel, payment }) => ( +const PaymentFailedView = ({ channel, nav }) => ( @@ -41,7 +41,10 @@ const PaymentFailedView = ({ channel, payment }) => ( channel.initCreate()}> Create channel - @@ -50,7 +53,7 @@ const PaymentFailedView = ({ channel, payment }) => ( PaymentFailedView.propTypes = { channel: PropTypes.object.isRequired, - payment: PropTypes.object.isRequired, + nav: PropTypes.object.isRequired, }; export default observer(PaymentFailedView); diff --git a/stories/screen-story.js b/stories/screen-story.js index 6de7b8b05..689b31a1b 100644 --- a/stories/screen-story.js +++ b/stories/screen-story.js @@ -149,9 +149,7 @@ storiesOf('Screens', module) .add('Pay Lightning Done', () => ( )) - .add('Payment Failed', () => ( - - )) + .add('Payment Failed', () => ) .add('Pay Bitcoin', () => ( ))