Node.js mysql error handling in promise

Question

I'm using node.js and express, also mysql. I use a connection pool to request connections and create a promise on it, to limit callback nightmare, the following snippet is set in a file that I import later, note that that I set an handler on error to not terminate the application in case of anything going really wrong

exports.getConnection = () => {
    return new Promise((resolve, reject) => {
        pool.getConnection((err, connection) => {
            if (err) {
                reject(`Could not obtain the connection from the pool: ${err}`);
            }
            connection.on('error', err => {
                console.log(`SQL error (code: ${err.code}, message: ${err.sqlMessage}) while executing query: ${err.sql}`);
            });
            resolve(connection);
        });
    });
};

And here is an example of usecase (the idea is to get the connection, chain the query in the then, and if a non fatal error happen I will throw it and handle the connection release in the catch handler

// Exception handler that release the connection then call the callback
function releaseConnectionHandler(err, connection, callback) {
    connection.release();
    callback(err, null);
}
exports.someRequest = function(ID, callback) {
    sqlPool.getConnection().then(connection => {
        connection.query("SELECT * from tableNotExists",
                        (err, result) => {
            if (err) { 
                throw ({ err, connection, callback }); 
            }
            connection.release();
            callback(null, result);
            });
    }).catch(({ err, connection, callback}) => releaseConnectionHandler(err, connection, callback));
};

The query will fail, but I see that the handler is not even called (I put some trace in it...) and the application terminates on

node_modules/mysql/lib/protocol/Parser.js:80
        throw err; // Rethrow non-MySQL errors

Correct querie yoeld no troubles...Any ideas what I did wrong on the error handling ?

Instead of throwing the error, you should reject the promise. Also, your code above mixes single and double quotes on your query. — Mike Atkins
– Mike Atkins, Commented Mar 31, 2018 at 11:33

Sven · Accepted Answer · 2018-04-01 01:15:44Z

2

You're re-throwing the error passed to the callback of your query, which the library you're using then re-throws as well, and finally isn't properly caught and handled anywhere and results in a failure. You're not in the context of the Promise when you throw, but the context of the callback function called from the mysql module.

You're also unnecessarily mixing promises and callbacks, in particular the function you're exporting. Your question indicates that you want to move away from callbacks, so I'm going to base this answer on that indication.

To solve the main issue, don't throw the error. Instead, pass it up to the callee:

const promisify = require("util").promisify;

exports.someRequest = function (ID) {
  return sqlPool.getConnection().then(connection => {
    return promisify(connection.query)("select * from tableNotExist")
      .finally(connection.release);
  });
};

The connection will always be released back to the pool, whether successful or on error. You can then call the method with:

yourModule.someRequest(id).then((results) => {
  // Do something with the result set
}).catch((e) => {
  // Handle error. Can be either a pool connection error or a query error.
});

If you have the possibility to use async/await, the code can be rewritten:

const promisify = require("util").promisify;

exports.someRequest = async function (ID) {
  let connection = await sqlPool.getConnection();
  try {
    return await promisify(connection.query)("select * from tableNotExist");
  } finally {
    connection.release();
  }
};

I also recommend using node-mysql2 since they have a Promise-based API in addition to their callback-style API and in my experience better performance as well. Then you don't have to write these tedious wrappers and instead just require('mysql2/promise') and be good to go.

edited Apr 1, 2018 at 1:15

answered Mar 31, 2018 at 11:37

Sven

5,27532 silver badges53 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

zebullon Over a year ago

As an additional question, why would the return promise still remember the enclosing try{...} finally {...} block ? Coming from C++ this seems very weird, I would not think this has anything to do with closure, since AFAIK this deals with enclosing variables not try-catch-finally specifiers ?

Sven Over a year ago

@zebullon Apologies, I missed adding an await before returning it. The previous solution did have a bug that would release the connection back to the pool before the query was complete. With the await expression there now, it will first wait for the query to complete before executing the finally-block (and returning the value). Does that make sense?

Sven Over a year ago

I should also explain the reasoning behind adding try/finally-clauses to this function. If the connection.query() throw an error, execution stops there and the error propagates up to the callee, meaning the connection.release() would never be executed. Wrapping the two calls in a try and finally-clause ensures that the connection.release()-method gets called even on an error. However, if sqlPool.getConnection() errors, we don't have a connection to release back to the pool, hence why it's put outside the try-clause.

Sven Over a year ago

If you're unsure about the execution flow, I've setup a repl for you here. Study and run the three functions getData, getDataConnectionError and getDataQueryError to better understand.

Collectives™ on Stack Overflow

Node.js mysql error handling in promise

1 Answer 1

4 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

4 Comments

Your Answer

Sign up or log in

Post as a guest

Related