Lnd manager

[PirosB3]

This is a stepping stone in order to tackle #162. As of today, if the lightning RPC service disconnects, there is no way for the client to reconnect automatically and the only solution is to restart the server. This PR tries to address that by providing a wrapper around lightning that handles internal state

What does this PR address (in order of relevance)

• refactors lightning into LightningManager class
• refactors app/routes to use an instance of LightningManager, using a small adaptor
• refactors other parts of the codebase to use an instance of LightningManager
• adds documentation to the code refactored
• reduces code duplication between server and server-le

Advice for reviewing this PR, where to start

1. Start with LightningManager, and review how the external API (.call) is used
2. Review lightningRPCAdapter function
3. Review all the function signatures in app/routes to make sure they were refactored appropriately
4. Review makeLightningManager server function
5. Do a dry test of the application, all functionality should work

Apologies if I didn't respect a specific stile guide, since none was enforced in README or in the files. I'm definitely happy to refactor stile/documentation/anything to fit the projects needs. Let me know if I should be doing something differently :) I will happily adapt

[mably]

Looks good to me besides a few indentations that need to be fixed.

[PirosB3]

@mably thanks for the review. As suggested by you, I have implemented the Airbnb Style Guide that we will be using from now onwards (including the files in this PR). If you don't have any technical comments I would suggest:

1. merge the PR
2. I will open a PR containing the linting to master. This code not only provides the configuration for the style guide, but it also reformats app/lightning.js and app/routes using the new style guide.

Alternatively, I can close this PR and open one containing the style changes for these files, but the line count to review go up 2x :(

[mably]

Merged. Thanks for your great work!

[mably]

Ok, just deployed it on some of my node and it looks like a few things are broken 😢

[mably]

@PirosB3 looks like error handling needs to be improved quickly, every error returns UNCATEGORIZED now.

[PirosB3]

I'm sorry to hear this. I'll gladly fix this. Do you have some specific spec you would like me to follow? What error messages do not want to categorize? Would you mind opening a ticket, assigned to me, with the above answer and also the other bugs you found? Thanks!

On Sat, Sep 29, 2018 at 4:52 AM François ***@***.***> wrote: @PirosB3 <https://github.com/PirosB3> looks like error handling needs to be improved quickly, every error returns UNCATEGORIZED now. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#163 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAztS3HsqTTupnnrM0R7R5amb2JCoL5Rks5uf18FgaJpZM4W1lqg> .

-- ************* PirosB3 https://github.com/PirosB3

[mably]

You just need to handle the gRPC as they were handled before, so the web front-end can display the correct error message to the user instead of a generic UNCATEGORIZED one.

[PirosB3]

Sounds great. I can work on it this weekend at some point (out of town). Does that timeline work for you?

On Sat, Sep 29, 2018 at 7:21 AM François ***@***.***> wrote: You just need to handle the gRPC as they were handled before, so the web front-end can display the correct error message to the user instead of a generic UNCATEGORIZED one. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#163 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAztS2OH2KihSxrtEU0K7CUdDvgmLlIaks5uf4IGgaJpZM4W1lqg> .

-- ************* PirosB3 https://github.com/PirosB3

[mably]

@PirosB3 looks like we are losing the original gRPC error here.
We need it to be able to handle it correctly on the web-client side.

[PirosB3]

Yup. I know exactly where to fix it.

On Sat, Sep 29, 2018 at 7:45 AM François ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In app/lightning.js <#163 (comment)>: > - logger.error("Required lnd certificate path missing from application configuration."); - process.exit(1); + switch(err.code) { + case grpc.status.UNIMPLEMENTED: + reject(LightningError.WALLET_LOCKED); + break; + case grpc.status.UNAVAILABLE: + reject(LightningError.NODE_UNREACHABLE); + break; + default: + logger.error("Unrecognized gRPC error: " + err); + reject(LightningError.UNCATEGORIZED); @PirosB3 <https://github.com/PirosB3> looks like we are losing the original gRPC error here. We need it to be able to handle it correctly on the web-client side. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#163 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAztS3ZrR66MPEhS4c0IpEbPlQhaBoPYks5uf4ejgaJpZM4W1lqg> .

-- ************* PirosB3 https://github.com/PirosB3

[mably]

@PirosB3 seems a bit excessive to me to drop the client on every gRPC error we are receiving 🤔

[mably]

Ok, no pb, you can work on it next week, I already fixed the most urgent issues.

[mably]

Pushed a quick hack to allow display of API errors in the web client: #177