node-mysql2
node-mysql2 copied to clipboard
ECONNRESET when closing SSL connection not being handled to NodeJS Error spec
Hi there, I receive the following error when closing a SSL connection that bubbles up:
{
errno: -4077,
code: "ECONNRESET",
syscall: "read",
stack:
"Error: read ECONNRESET\n at TLSWrap.onStreamRead (node:internal/stream_base_commons:220:20)\n at TLSWrap.callbackTrampoline (node:internal/async_hooks:130:17)",
message: "read ECONNRESET",
}
I've narrowed it down to the following lines that are mishandling the exception thrown:
// Line 160 of mysql2/lib/connection.js
_handleNetworkError(err) {
if (this.connectTimeout) {
Timers.clearTimeout(this.connectTimeout);
this.connectTimeout = null;
}
// Do not throw an error when a connection ends with a RST,ACK packet
if (err.errno === 'ECONNRESET' && this._closing) {
return;
}
this._handleFatalError(err);
}
Issue seems to be that it's looking for err.errno
as a string. However, errno
has exclusively been a negative number since Node 13, with err.code
being the string that would emit ECONNRESET
.
I see that this has been fixed in master. Can this please be released to npm?
@sidorares Is the fix planned for release anytime soon to NPM? If not, is there a reason?
Just met this one myself. Thankfully the patch-package utility exists so I can fix it for my codebase and move on.
Here's my patch: mysql2_2.3.3.patch
diff --git a/node_modules/mysql2/lib/connection.js b/node_modules/mysql2/lib/connection.js
index 47970e9..4704db5 100644
--- a/node_modules/mysql2/lib/connection.js
+++ b/node_modules/mysql2/lib/connection.js
@@ -174,7 +174,7 @@ class Connection extends EventEmitter {
this.connectTimeout = null;
}
// Do not throw an error when a connection ends with a RST,ACK packet
- if (err.errno === 'ECONNRESET' && this._closing) {
+ if (err.code === 'ECONNRESET' && this._closing) {
return;
}
this._handleFatalError(err);
Just met this one myself. Thankfully the patch-package utility exists so I can fix it for my codebase and move on.
Here's my patch:
mysql2_2.3.3.patch
diff --git a/node_modules/mysql2/lib/connection.js b/node_modules/mysql2/lib/connection.js index 47970e9..4704db5 100644 --- a/node_modules/mysql2/lib/connection.js +++ b/node_modules/mysql2/lib/connection.js @@ -174,7 +174,7 @@ class Connection extends EventEmitter { this.connectTimeout = null; } // Do not throw an error when a connection ends with a RST,ACK packet - if (err.errno === 'ECONNRESET' && this._closing) { + if (err.code === 'ECONNRESET' && this._closing) { return; } this._handleFatalError(err);
That looks useful. I'll have to look into it further. Thanks for the heads up!
@kpervin @kf6kjg the fix should be in master ( #1438 ) but is not published yet ( not sure why, last release published after PR was merged. need to investigate that )
would be good to add an integration test for this
@kpervin @kf6kjg the fix should be in master ( #1438 ) but is not published yet ( not sure why, last release published after PR was merged. need to investigate that )
Any news regarding this being published to NPM?
@sidorares Will this be pushed to NPM? At present I am having to patch the repo via yarn patch
in the meantime.