fix the issue 890 which throw exception Cannot destructure property '…

[benzhuo]

…message' of 'tracker.fetch(...)' as it is null

[jsumners]

Thank you for contributing. We will need a test that fails as described in #890 without this change. My intention is to write an issue specific test that uses ldapjs.createServer to create a "server" that will induce the problem in the client. It would be similar to the test https://github.com/ldapjs/node-ldapjs/blob/f2890088e49c0c7b7b880998d73d6e4a448d7b4e/test/issue-845.test.js. I just haven't been able to dedicate the time to it yet.

[benzhuo]

I am not very understanding, this is a very explicit syntax error.
but it is a critical issue and affects production

[jsumners]

We need to include a unit test that covers the issue in order to guard against regression. I have provided guidance on how I intend to approach the test. I can provide further guidance if you have specific questions, or you can wait until I have time to get to it myself.

[benzhuo]

Hi Jsumners,
I try to write the unit test, added file issue-890.test.js and fixed the case of #fetch function in file index.test.js
but, you know I only have one way to recreate the issue that is to set the timeout to the a small number, like 5.
If I set it to 5, the timeout exception error always occurs which caused that it's hard to write a case to test my change.

Please help.

[jsumners]

We can set a timeout for the test like tap.test('handle null messages', { timeout: 2000 }, t => {. Then use a lower setTimeout in the server to delay the response past the time the connecting client would expect it.

At least, that's my idea for how to write this test.

[jsumners]

Thank you for working on this @benzhuo. Unfortunately, the included test does not cover the crashing case. The primary work in this PR is developing that test. So I am closing this PR in favor of #913 which includes a working test.