JS - Unhandled promise

Question

I created a node.js web application. Currently I'm using MSSQL for the DB. I added the node-mssql package. I created functions, in a separate file called sql.js, to perform the corresponding SQL functions. The controller.js calls them with the await keyword. I had no errors when i was make one request at a time, but if two request come in around the same time i get the following error.

(node:136648) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): ReferenceError: error is not defined
(node:136648) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

I think i has something to do with the connection, im using a global one, but i debugged my controller.js and saw that pool was defined, so im unsure what is causing the issue.

SQL.js

const sql = require('mssql'); 
const config = {
    user: 'admin',
    password: 'linux',
    server: '11.222.33.44',
    database: 'master'
}
const pool = new sql.ConnectionPool(config); 
pool.on('error', err => {
    if (err) {
        console.log('sql errors', err);
    }
    if (!err) {
        pool.connect();
    }
});
module.exports.getUsers = async() => {
    // const pool = await sql.connect(config);
    try{
        const result  = await pool.request()
            .query(`SELECT * FROM master.dbo.users ORDER BY user_id`)
        // sql.close();
        return result.recordset; 
    }catch(err){
        throw error; 
    }
}
module.exports.getProducts = async() => {
    // const pool = await sql.connect(config);
    try{
        const result  = await pool.request()
            .query(`SELECT * FROM master.dbo.products ORDER BY product_zip`)
        // sql.close();
        return result.recordset; 
    }catch(err){
        throw error; 
    }
}

controller.js

const express = require('express');
const {getInfo, getUsers, getProducts} = require('../lib/sql');
const router = express.Router();

...

/// Get Users
router.get('/controller/getUsers', async(req, res, next) => {
    console.log('get: /controller/getUsers'); 
    const users = await getUsers(); 
    res.status(200).json(users);   
});
/// Get Products
router.get('/controller/getProducts', async(req, res, next) => {
    console.log('get: /controller/getProducts'); 
    const products = await getProducts(); 
    res.status(200).json(products);
});
module.exports = router;

Answer 1

There is no error handling in your various router.get callbacks, and router.get doesn't do anything with the return value of the function you pass it, so if you pass it an async function, the promise that function returns will not be handled, and for rejected promises, that's an unhandled error. Hence the error message.

Instead: You have to handle errors. For instance:

router.get('/controller/getUsers', async(req, res, next) => {
    try {
        console.log('get: /controller/getUsers'); 
        const users = await getUsers(); 
        res.status(200).json(templates);
    } catch (e) {
        res.status(500).send(/*...*/);
    }
});

That still returns a promise that's never handled, but at least it never rejects (assuming nothing in the catch code fails).

If you have a common way of returning errors, you probably want to give yourself a wrapper function around router.get (and such) that handles errors from the callbacks so that's nicely centralized.

For instance:

const asyncRouterGet = (route, handler) => {
    return router.get(route, (req, res, next) => {
        handler(req, res, next)
        .catch(err => {
            // Common error handling here
            res.status(500).send(/*...*/);
        });
    });
};

then

asyncRouterGet('/controller/getUsers', async(req, res, next) => {
    console.log('get: /controller/getUsers'); 
    const users = await getUsers(); 
    res.status(200).json(templates);
});

...since you know rejections will be automatically handled by asyncRouterGet's generated callback.

---

And obviously, don't do this:

try {
    // ...
}
catch (err) {
    throw error;
}

error isn't defined anywhere in your quoted code, and

try {
    // ...
}
catch (err) {
    throw err;
}

would be pointless; just leave the try/catch off if you're going to rethrow without doing anything else.