Implement retriable connection

[sebastienfilion]

Add a test for an issue that was reported where the process exits if the connection to the Redis server is lost while being subscribed to a topic.

To run this test, you will need to specifically allow to run (execute) commands and enable the unstable API -- deno test --allow-net --allow-run --unstable

Reference: #83

[sebastienfilion]

At the moment, a client will die if the server is killed or the connection is lost. This is a problem especially with clients subscribed to a channel.
Since the Writer/Reader buffers are passed around to I/O functions, it is impossible to reconnect a Redis client.
Having a connection object allows to easily replace the buffers.

Resolves: #83

ToDo

• Fix 3 failing tests
• Update documentation on retriable connections

The following tests are failing inconsistently, and I would need some help to figure out why:

• testSubscribe
• connect with wrong password
• connect with empty password

For example, before running testSubscribe (only when running all the tests; never if running only the tests under ./pubsub_test.ts); the Redis client loops on DB index from 0 to 15 and then flush wherever it ends... In my case, it selects up to 11 and doesn't flush... Something seems to go wrong and makes the before-mentioned failing tests...

1592932362.977302 [0 127.0.0.1:60779] "SELECT" "1"
1592932362.977361 [0 127.0.0.1:60780] "SELECT" "2"
1592932362.977377 [0 127.0.0.1:60781] "SELECT" "3"
1592932362.977384 [3 127.0.0.1:60781] "SELECT" "3"
1592932362.977388 [3 127.0.0.1:60781] "FLUSHDB"
1592932362.977402 [0 127.0.0.1:60782] "SELECT" "4"
1592932362.977414 [0 127.0.0.1:60783] "SELECT" "5"
1592932362.977552 [0 127.0.0.1:60784] "SELECT" "6"
1592932362.977574 [0 127.0.0.1:60785] "SELECT" "7"
1592932362.977585 [0 127.0.0.1:60786] "SELECT" "8"
1592932362.977595 [0 127.0.0.1:60787] "SELECT" "9"
1592932362.977604 [0 127.0.0.1:60788] "SELECT" "10"
1592932362.977614 [0 127.0.0.1:60789] "SELECT" "11"

I would like some help to finish this PR.

[uki00a]

Thanks for working this, @sebastienfilion!

[uki00a]

Umm, it looks like the testSubscribe case is causing a resource leak 🤔
I'll also look into the cause of this.

$ deno test -A --unstable --filter testSubscribe
running 4 tests
test testSubscribe ... FAILED (9ms)
test testSubscribe2 ... ok (6ms)
test testSubscribe3 ... ok (5ms)
test testSubscribe4 (#83) ... ignored (0ms)

failures:

testSubscribe
AssertionError: Test case is leaking async ops.
Before:
  - dispatched: 23
  - completed: 12
After:
  - dispatched: 44
  - completed: 44

Make sure to await all promises returned from Deno APIs before
finishing test case.
    at Object.assert ($deno$/util.ts:35:11)
    at asyncOpSanitizer ($deno$/testing.ts:43:5)
    at async Object.resourceSanitizer [as fn] ($deno$/testing.ts:67:5)
    at async TestRunner.[Symbol.asyncIterator] ($deno$/testing.ts:275:11)
    at async Object.runTests ($deno$/testing.ts:358:20)

failures:

	testSubscribe

test result: FAILED. 2 passed; 1 failed; 1 ignored; 0 measured; 177 filtered out 

[sebastienfilion]

Umm, it looks like the testSubscribe case is causing a resource leak 🤔
I'll also look into the cause of this.

Part of the problem is ./tests/test_util.ts that creates multiple clients (about 12) at runtime and never seems to close them.

If I run deno test [...] pubsub_test.ts the tests complete without any leak (this test doesn't import the utilities).
But if I run deno test [..] . the tests fails because of the left over TCP streams...

I guess that it's because before, the TCP connection was garbage collected with the writers?

[uki00a]

@sebastienfilion
Thank you for submitting the changes!
There are still a few things that need to be fixed.
Please check again.

[uki00a]

Thanks for submitting the changes @sebastienfilion!

Fix 3 failing tests

We can merge this PR when the above issue is resolved.
Please wait a little longer.

[sebastienfilion]

We can merge this PR when the above issue is resolved.

Will you help resolve the issue or you want me to do it?

[uki00a]

Yeah, I'll also investigate the cause.

[uki00a]

Requested changes address the problem of the testSubscribe case.
(However, the other cases still fail...)

[sebastienfilion]

Requested changes address the problem of the testSubscribe case.
(However, the other cases still fail...)

Good catch!

[uki00a]

@sebastienfilion
I commented again. Please fix them!
Then all the tests should pass.

[sebastienfilion]

@uki00a I made the changes that you suggested and all but one test passes! 🎈
This one is not working connect with empty password because either the TypeScript compiler will complain that password can't be any or else at runtime TypeScript will catch that the password is empty and will not throw ErrorReplyError as the test expect.
So either I force the compiler to allow a password to be any or a change the error from ErrorReplyError to TypeError (?)

[uki00a]

Awesome work!
Thanks @sebastienfilion!

[uki00a]

Released in v0.11.0!