MYSQL2 Dialect - Connection not disposed after a connection error during query execution

[davidf84]

Environment

Knex version: 0.21.21 (same issue on 0.95.12)
Database + version: MYSQL 5.7
OS: Tested on Windows 10 and Linux Ubuntu 20.04.2

Bug

1. When there is a connection issue (closed connection) during a query execution, the connection does not get removed from the pool and continues to be used, causing all queries executed after to fail with connection is closed error. The only way to recover from the issue is to restart the app. Note this does not happen on mysql dialect, only on Mysql2 dialect.

MYSQL2 does not emit an error event, when there is a connection issue while executing a query, but returns a fatal=true property in the exception. The dialect should handle this as a fatal error and set the __knex__disposed ?

2. Error: Can't add new command when connection is in closed state

3. Example Code (needs a mysql server)

const knex = require('knex')({
    client: 'mysql2',
    connection: {
      host : '127.0.0.1',
      port : 3306,
      user : 'your_database_user',
      password : 'your_database_password',
      database : 'myapp_test'
    },
    pool: { min: 0, max: 7 }
});

const main = async function()
{
    try {
        await knex.raw("select sleep(100)");
        // !! Manually KILL Connection from MYSQL to simulate connection error => KILL <pid>
    } catch(err) {
        console.log(err);
    }

    // all queries from now on will return error when reusing the same connection in the pool
    try {
        await knex.raw("select 1");
    } catch(err) {
        console.log(err);
    }
    process.exit();
}
main();

[christianjank]

@davidf84 This looks like what I've been running into (let me know if you think this issue is different).

While debugging I've noticed that the validateConnection function does not check for the _protocolError property. See:

knex/lib/dialects/mysql2/index.js

Lines 17 to 23 in fdad316

	validateConnection(connection) {
	if (connection._fatalError) {
	return false;
	}
	return true;
	}
	}

I've been able to fix it by replacing the function at runtime with something like the following:

  knex.client.validateConnection = (conn) => {
    if (connection.__fatalError || connection._protocolError) {
      return false
    } else {
      return true
    }
  }

While googling I came across the code that sequelize was using and they seem to be doing this, as well as checking _closing and stream.destroyed. See: https://github.com/sequelize/sequelize/blob/9f950cbcbdd659d559496b77c40e0f827b108561/lib/dialects/mysql/connection-manager.js#L141-L148

[davidf84]

Hi @christianjank, yes that is the same issue and that solves it. At least we have a temporary fix until this is fixed in the library.

Thanks for your help.

[kibertoad]

@davidf84 Would you consider sending PR for this issue?

[OlivierCavadenti]

@christianjank thanks for the solution, it's an easy fix.

@davidf84 Would you consider sending PR for this issue?

I was just on it when you send your message.

[kibertoad]

Fix released in 0.95.14