alexbosworth / ln-service

Node.js interface to LND
MIT License
319 stars 60 forks source link

need to be able to unregister event #122

Open nicolasburtey opened 4 years ago

nicolasburtey commented 4 years ago

I'm using jest with subscribeToChainAddress

One of the issues I'm running into is that subscribeToChainAddress is creating a eventEmitter internally, and add some event listener to it.

Because those event listeners are ending up in the event queue, I'm not able to exist Jest properly, jest instead is timing out.

I believe one way that could deal with it is by returning this emitter (https://github.com/alexbosworth/ln-service/blob/44b0d6baeb4cfba16a0ecaf1cd17778a183c012f/chain/subscribe_to_chain_address.js#L71) so a subInternal.removeAllListeners() could be launched once the tests has passed.

alexbosworth commented 4 years ago

Ah ok I'll look at this

alexbosworth commented 4 years ago

I tried an approach to fix this, pushed to master and a new version, does this fix it for you?

nicolasburtey commented 4 years ago

that works. thanks!

nicolasburtey commented 4 years ago

I'm actually running into the same issue with subscribeToChannels.

alexbosworth commented 4 years ago

Aha I can take a look at adding the same solution there

nicolasburtey commented 4 years ago

this would be awesome :)

alexbosworth commented 4 years ago

this would be awesome :)

I pushed the same thing for subscribe to channels to master, is this working for you?

nicolasburtey commented 4 years ago

Jest is giving me this weird error:

1 CANCELLED: Cancelled

      at Object.exports.createStatusError (node_modules/grpc/src/common.js:91:15)
      at ClientReadableStream._emitStatusIfDone (node_modules/grpc/src/client.js:233:26)
      at ClientReadableStream._readsDone (node_modules/grpc/src/client.js:199:8)
      at node_modules/grpc/src/client_interceptors.js:683:15

Trying to investigate where that could come from.

as a side note, I have added ln-service with : yarn add https://github.com/alexbosworth/ln-service#master and looking at yarn.lock seems to have fetch the right commit. so don't think that it is related.

nicolasburtey commented 4 years ago

if I comment a line with once(sub, 'channel_opened'), then the error disappeared. But for some reason, the error was not here with the last tagged version. weird.

any clue about what this could be?

alexbosworth commented 4 years ago

Is the issue that it is showing the error or is it still not exiting properly?

alexbosworth commented 4 years ago

I have another update to master that I think can deal with this issue, can you try that?

alexbosworth commented 4 years ago

Pushed the change to master, are there other subscription APIs that you think should follow suit?

nicolasburtey commented 4 years ago

I have another similar issue with payViaRoutes. I'm doing somthing like this:

const promise = lnService.payViaRoutes({ lnd: this.lnd, routes: [route], id })
await Promise.race([promise, timeout(TIMEOUT_PAYMENT, 'Timeout')])

so that if the invoice is not settled quickly, the front end is not stuck forever.

but if this goes the timeout path, jest doesn't like it, as if the promise never completed. I guess it's a similar issue as above?

alexbosworth commented 4 years ago

Which promise isn't completed? The payViaRoutes promise never returns at all?

nicolasburtey commented 4 years ago

if the node receiving the payment hodl the invoice, and the timeout occurs, my understanding is payViaRoutes might have open handlers that will make jest complain

alexbosworth commented 4 years ago

if the node receiving the payment hodl the invoice, and the timeout occurs, my understanding is payViaRoutes might have open handlers that will make jest complain

Did you try canceling the hodl just after the timeout?

nicolasburtey commented 4 years ago

I tried settleHodlInvoice after the timeout but have the same behavior

alexbosworth commented 4 years ago

I tried settleHodlInvoice after the timeout but have the same behavior

Is it possible to introduce a delay on the test side before marking the test as complete?

Do you have similar issues with subscribeToPayViaRoutes?

nicolasburtey commented 4 years ago

I'll try both of those

alexbosworth commented 4 years ago

This call is different from the normal subscription calls because it is not a subscription on the LND side

nicolasburtey commented 4 years ago

subscribeToPayViaRoutes or payViaRoutes?

alexbosworth commented 4 years ago

subscribeToPayViaRoutes or payViaRoutes?

Neither has a real subscription to LND, both are simply waiting for a response from LND to continue

Real subscriptions to LND can be canceled, requests just wait for a return value