Pool option "enableKeepAlive" is ignored?

[davidecaroselli]

Hi all!

I was experiencing several issues with Pool in production due to read ECONNRESET error and I finally found the response in this Github Issue: #683 (and, by the way, thanks to all for the amazing work done and the patience in debugging it! ;) )

So, I have updated from 2.0.1 to 2.2.5 (latest right now), but I found something strange. Looking at the code it seems that the new PoolConfig property enableKeepAlive is never actually queried.
I found this piece of code that I think should read this property:

node-mysql2/lib/connection.js

Line 42 in 2be0eb6

this.stream.setKeepAlive(true, this.config.keepAliveInitialDelay);

But as you can see the function is always called with a hardcoded true.
Now, this is fine with me, because I was about to enable it anyway, but my question is: is this intentional? Am I missing some obvious bit of code?

Thanks!

[sidorares]

hm, looks like this is side effect of #1086 . see #1081 (comment) for reasons why it was added but to me looks like it should be moved inside if (this.config.enableKeepAlive) {} block

[kn3ny]

I have the same doubt as @davidecaroselli during I was reading the code.
I believe the pull request #1258 will fix this issue.