h0x91b / redis-fast-driver

78 stars 13 forks source link

feat(redis): implement initial connection timeout #43

Closed STRML closed 3 years ago

STRML commented 3 years ago

Occasionally I've seen redis simply fail to connect, or a slow network / dropped packet can cause this.

Tests added to match.

h0x91b commented 3 years ago

Looks good, I'll merge and push it when will have a time

Thanks

h0x91b commented 3 years ago

npm tests fail, something breaks tests

      ✔ works correctly when send buffer needs to be resized
      ✔ works correctly when command buffer needs to be resized
      errors
        ✔ incr a string
        ✔ zrange string
    queueing
      ✔ Queues up messages before connect
      ✔ Releases queue on end

  23 passing (172ms)

/Users/arsenyp/Desktop/src/redis-fast-driver/node_modules/mocha/lib/runner.js:965
    throw err;
    ^

Error: Connection Timeout.
    at onError (/Users/arsenyp/Desktop/src/redis-fast-driver/index.js:49:26)
    at Timeout._onTimeout (/Users/arsenyp/Desktop/src/redis-fast-driver/index.js:57:9)
    at listOnTimeout (internal/timers.js:554:17)
    at processTimers (internal/timers.js:497:7)
Emitted 'error' event on Redis instance at:
    at onError (/Users/arsenyp/Desktop/src/redis-fast-driver/index.js:49:12)
    at Timeout._onTimeout (/Users/arsenyp/Desktop/src/redis-fast-driver/index.js:57:9)
    at listOnTimeout (internal/timers.js:554:17)
    at processTimers (internal/timers.js:497:7)
npm ERR! Test failed.  See above for more details.

probably somewhere missing clearTimeout

STRML commented 3 years ago

Appears this is caused by the preceding spec, Disconnects if ended during connect (potential race condition). Will open a new PR.