Light-client: Integration tests don't exit immediately

[nephix]

Steps to Reproduce

1. Start docker
   2.

$ cd light-client/integration
$ ./run-integration.sh

Expected Result

Jest exits immediately after the tests are done

Actual Result

Jest keeps running, sometimes for almost a minute

Test Suites: 1 passed, 1 total
Tests: 3 passed, 3 total
Snapshots: 0 total
Time: 92.321s
Ran all test suites.
Jest did not exit one second after the test run has completed.

This usually means that there are asynchronous operations that weren't stopped in your tests. Consider running Jest with --detectOpenHandles to troubleshoot this issue.

[nephix]

Maybe related to https://github.com/matrix-org/matrix-js-sdk/issues/1101

[kelsos]

So up to this point, we know that if the --detectOpenHandles flag is used, there is no error, no open handles are getting logged and the tests terminate properly.

I did some investigation but I could not figure out the actual cause of this issue. One of the things tried was monkey patching the request library inside Matrix to call the abort() method on the requests by hand. Unfortunately that did not work at all.

After a lot of unsuccessful attempts I found https://github.com/Raynos/leaked-handles which I used to detect any open handles. I could find only two reported file descriptors that kept pointing to /dev/pts/0 which doesn't make any sense to me.

I did a search but I could not find more information.

[nephix]

I also tried a bunch of stuff, like

• Fiddling with abort-controller because we use abort-controller/polyfill which only seems to polyfill IE when bundling (which we don't do in tests)
• Trying to find open handles

I also skipped a couple of integration tests (minting and channel opening) and the waiting time at the end was still there, although it was a lot shorter.

Since the issue seems to be difficult to resolve and the tests work, I'd say lets put this in the back log and revisit it once the waiting time becomes an actual issue

[andrevmatos]

So I found the culprit:
As I had suggested, being mostly sure matrix was the issue, I used matrix-js-sdk's request function to replace/wrap matrix internal request with a function which would log on call (start), callback (result) and abort (shutdown);
I used mostly the cli, which upon first Ctrl+C calls stop, and upon 2nd exits immediately, and did suffer from this issue as well (as it's also a Node app as much as jest), in the form of the first Ctrl+C hanging and not exiting for up to almost 2min.
With the verbose wrapper above, I noticed abort for the same sync request (the current one at shutdown time) was being called twice, which rang some bells. Adding some console.trace to abort have shown that the first one is called from SyncApi.stop, called from stopClient, as it should, but the second one was called by this line.
So, as evidenced by it, it's not the request which hangs (as it is properly called and aborted on first call), but that resetTimeout timeout which was pending, and gets cleared only when the request's callback is called (which it wasn't on abort);
So the fix was simply patching abort with an implementation that also calls the callback with some generic error upon abort, to ensure those timeouts are properly cleared. This is harmless on browsers, as it doesn't wait for pending timeouts the same way Node apps do, and doesn't trigger weird unhandled errors because request errors are something common and expected by matrix or any client everywhere.

[kelsos]

@andrevmatos Did you also try the integration tests to see that this also solves the issue for the tests?

[andrevmatos]

Yes

[kelsos]

Awesome thank you very much :) I already approved the PR. Now I can sleep peacefully :D