search

getAlby / js-lightning-tools

Collection of helpful building blocks and tools to develop Bitcoin Lightning web apps
https://www.npmjs.com/package/@getalby/lightning-tools
MIT License
34 stars 14 forks source link

fix: implement network error handling for verify payment #154

Closed Toheeb-Ojuolape closed 1 month ago

Toheeb-Ojuolape commented 1 month ago

What type of PR is this? (check all applicable)

Description

This PR implements a try-and-catch block for the verifyPayment method in invoice.ts and returns false if a network error occurs

Related Tickets & Documents

Resolves https://github.com/getAlby/js-lightning-tools/issues/125

Closes https://github.com/getAlby/js-lightning-tools/issues/155

Toheeb-Ojuolape commented 1 month ago

Hi @rolznz this is the new PR for the issue where I made a commit using the --no-verify flag which did not result in a change in the yarn.lock. I also implemented your error code corrections and I hope you find this acceptable. Thank you.

rolznz commented 1 month ago

Thanks @Toheeb-Ojuolape it looks great!

Any chance you would be able to write a test too? if not, that's fine.

Toheeb-Ojuolape commented 1 month ago

Thanks @Toheeb-Ojuolape it looks great!

Any chance you would be able to write a test too? if not, that's fine.

Hi @rolznz , sorry I won't be able to write a test for it as that would require me to mock both verifyPayment and the network errors which is out of the scope of this current issue. I can add test suites later on if there is an issue created related to unit tests for verifyPayment, validatePreimage and all the other async functions in invoice.ts.

I hope you find this acceptable 🙂

Toheeb-Ojuolape commented 1 month ago

Hi @rolznz just following up on this. Once this PR has been merged, I'll create a new issue for writing tests for async functions like verifyPayment, isPaid and validatePreimage in the invoice.ts and assign the task to myself.

I look forward to your feedback.

Toheeb-Ojuolape commented 1 month ago

Hi @rolznz, my PR is still here for your approval 🥺. Any chance this Friday is a good time to approve?

I have already created a new issue for tests in invoice.ts which I plan to address once I have closed out on this: https://github.com/getAlby/js-lightning-tools/issues/155

rolznz commented 1 month ago

@Toheeb-Ojuolape I added some tests. Thanks!