lnc: Add a timeout when creating the grpc client with the mailbox #127
Conversation
positiveblue changed the title 
guggero left a comment
guggero left a comment There was a problem hiding this comment.
Thanks for the fix! Code looks good, just a question about context lifetime.
| ctxt, cancelT := context.WithTimeout( | ||
| context.Background(), DefaultConnectionTimetout, | ||
| ) | ||
| defer cancelT() |
There was a problem hiding this comment.
Are you sure we can cancel this context here? Is the context only used for the initial dialing and not for the whole duration of the connection?
There was a problem hiding this comment.
Good question, I had to check it before opening the PR because I wasn't sure neither.
Form the grpc docs:
In the blocking case, ctx can be used to cancel or expire the pending connection. Once this function returns, the cancellation and expiration of ctx will be noop. Users should call ClientConn.Close to terminate all the pending operations after this function returns.
Make sure that we do not hold forever if we are not able to dial with the maiblox service. This could happen in cases where a passphrase is being reused and the secret key has already been negotiated for example.
positiveblue force-pushed the fix-124 branch from ec4e929 to 5662500 Compare 
| @positiveblue the code looks good, thanks. Maybe we should also bump the LNC dependency to the recently released version |
The lates version of lightning-node-connect (`v0.3.0-alpha`) requires at least that version.
|
I also had to bump the minim go version for compiling the project because LNC needs |
2 participants
Do not hang forever when creating the grpc client with the mailbox.
Also, make sure we use the
DefaultStoretimeouts every time we call store functions in the package.Both timeouts are currently set to 10s.
Fix #124