Use mysql with promises in node.js

I just followed the tutorial for authentication on node from scotch.io. They used mongodb as a backend, and I ported this to mysql. This is my first node app connecting to a database.

I used bluebird to promisify the mysqljs/mysql package in config/database.js

const mysql = require('mysql');
const Promise = require("bluebird");
Promise.promisifyAll(mysql);
Promise.promisifyAll(require("mysql/lib/Connection").prototype);
Promise.promisifyAll(require("mysql/lib/Pool").prototype);

Here is the code of my model/user.js:

const mysqlPool = require('../../config/database'),
    bcrypt = require('bcrypt-nodejs'),
    Promise = require("bluebird");
    getDb = function(){
        return mysqlPool.getConnectionAsync();
    }
let user = {
    create(user, done) {
        getDb().then((db) => {
            return db.queryAsync(`insert into bot_users (name, token, type, external_id) values (?, ?, ?, ?)`,
                [user.name, user.token, user.type, user.external_id])
        }).then((results, fields) => {
            console.log(results, fields)
            user.id = results.insertId
            return done(null, user)
        })
    },

There is something that does not look good to me:

getDb().then((db) => {
    return db.queryAsync(...)
}).then(...)

looks somehow convoluted. I would like to have a variable db that I could use directly as db.queryAsync(...).then(...), but I'm not able to find out how to assign such a promise to the variable db.

Also I would like an advice on my approach. Is this a good approach to use promises here or is there a better one ?

Is it necessary to return done(null, user) inside the then callback in order to stick to the general node convention, or is this no longer required when using promises (bluebird) ?

Solution

The most important advice for any work with promises is: Ditch callbacks.

Your code currently is a hybrid between callbacks (done()) and promises. Don't do that. Use promises exclusively. The key point of promises is that you can return something from your async functions. You don't need to rely on the calling side passing in a callback.

Promisify all APIs. Emphasize on returning things.

const Promise = require("bluebird"),
    mysqlPool = Promise.promisifyAll(require('../../config/database')),
    bcrypt = Promise.promisifyAll(require('bcrypt-nodejs'));

let user = {
    create: params => mysqlPool
        .getConnectionAsync()
        .then(db => db.queryAsync(
            'insert into bot_users (name, token, type, external_id) values (?, ?, ?, ?)',
            [params.name, params.token, params.type, params.external_id]
        ))
        .then((results, fields) => ({
            name: params.name,
            token: params.token,
            type: params.type,
            external_id: params.external_id,
            id: results.insertId
        }))
};

Note how user.create() returns an unbroken chain of values. A mySQL connection turns into a query result turns into a new user object. I dislike overriding the existing instance. Avoid side-effects.

Usage is straight-forward.

user.create({
    name: "foo",
    token: 12345,
    type: "user",
    external_id: 67890
}).then(newUser => {
    console.log(newUser);
}).catch(err => {
    console.error(err);
});