Throw an error when a connection ends with a RST,ACK packet?

[alejandromav]

According to MySQL docs, COM_QUIT command should end with "either a connection close or a OK_Packet".

As @elemount pointed out in issue #1730, Azure MySQL is answering with an RST,ACK.

It can be reproduced when you run this example code against an Azure MySQL instance.

var mysql      = require('mysql');
var connection = mysql.createConnection({ config });

connection.connect();

connection.query('SELECT 1 + 1 AS solution', function (error, results, fields) {
  if (error) throw error;
  console.log('The solution is: ', results[0].solution);
});

connection.end();

Throws ECONNRESET error.

Should this library throw an error when a connection end with a RST with the ACK flag set?
Can we detect this?

[dougwilson]

Can you provide the fix or help me setup an Azure MySQL instance to run the code against?

[alejandromav]

Sure, please give me an email address and I'll send you the connections parameters.

[dougwilson]

Use the one on my GitHub profile :)

[dougwilson]

@alex030293 provided a connection to a MySQL Azure instance and I reproduced the issue and have a tentative fix. I should land the fix on master today once I get it cleaned up.

[dougwilson]

Sorry for the delay, I'm still trying to figure out how to actually make a test for this. It seems like Node.js does not treat a RST as an error unless it thinks it has something to read. It only seems to be an issue with the ACK from the sent data is combined with the RST, but if the ACK and RST are separate Node.js doesn't throw an error. I can't seem to get Node.js code to get them to emit together so far, through, in order to create a test.

[dougwilson]

@elemount I think in an issue somewhere you said Azure team would be willing to provide a server I can connect the CI of this project to. That would be awesome and definitely help out with this issue, which I cannot figure out how to get a working test for without having an Azure MySQL server in the test suite. Travis CI has the ability to use encrypted secrets in the config, so I can add the credentials to the CI without them being available to the public.

[elemount]

Let me email to him and let he connect with you.

[KalenAnson]

I have been chasing unhandled ECONNRESET errors related to the mysql library for a while now.

I am getting ECONNRESET unhandled errors from the Connection instance in the file mysql/lib/Connection.js on line 115. The errors look to be related to browser clients that go idle and get reset by the browser. Here is the offending line of code:

this._protocol.on('unhandledError', this._handleProtocolError.bind(this));

The ECONNRESET is a TCP socket event (error), and is handled by Connection._socket. I traced the logic and it appears that the RST packets cause a socket error that eventually bubbles up from the socket object to the protocol object and then back to the Connection instance as an unhandledError event.

What should the Connection instance do when ECONNRESET shows back up as an unhandledError event? It appears that by this point the Protocol queue has been purged and the connection is now in question. It might be nice to close the socket and let the Connection instance emit end rather than emitting an error. Thoughts?

[dougwilson]

Ok, I got the instructions for how to get the Azure MySQL setup, so I'll work on getting it running in our CI for this issue.

[alejandromav]

@dougwilson Any update on this? Should we expect this to be fixed in next release? Is there any ETA for next release?

Thanks

[dougwilson]

I have a failing branch pushed up right now. Still need to get Azure set up and finish the changes. This is what is holding the next release, so yes it is expected in the next release.

[dougwilson]

Here is the branch if you want to take a look / try: https://github.com/mysqljs/mysql/tree/quit-rst

[dougwilson]

I merged the change into master now. Please try master and validate that is resolves your issue, when you have a chance.