matrix-org / matrix-org/matrix-js-sdk
Node process crash when /sync times out (after calling stopClient)
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 2.2k
- Forks
- 704
- Avg merge
- 1d 20h
- Merged PRs (30d)
- 40
Description
Hi there, #2472 already reports that `stopClient` is non-functional in a Node.JS environment but as it turns out it's possible to crash the process if the /sync request fails due to a `ConnectionError`.
A typical scenario is:
- You run a integration test which spawns a homeserver.
- You start a js-sdk client and start syncing against the HS.
- Test completes, you stop the client and then kill the homeserver.
- The homeserver no longer exists, but because the /sync doesn't abort properly it will raise a ConnectionError about the TCP socket no longer being there.
- The Node.JS process crashes due to the ConnectionError not being handled.
As it stands, this is a bit of a showstopper because it means we cannot write integration tests for things which depend upon the js-sdk for our application. I (assume) it also means that if the homeserver were to go away during actual running of the application, there is a chance the process may also crash.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the stopClient and /sync flow described in the issue, reproducing the integration-test scenario where the homeserver is killed after the client stops. Trace how a ConnectionError from the failed request is handled. Done means stopping the client and losing the homeserver no longer crashes the Node.js process.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- node.js, typescript
- Domain
- api
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100