Kpizzle
Kpizzle

Reputation: 430

UnhandledPromiseRejectionWarning in Express App

I'm currently learning Javascript/Node.js/MEAN stack and I'm following an Express tutorial.

I get the following error:

(node:11524) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): TypeError: Cannot read property 'close' of undefined (node:11524) [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.

When I hit this route in my browser.

function router(nav) {
adminRouter.route('/')
    .get((req, res) => {
        const url = 'mongodb://localhost:27017';
        const dbName = 'libraryApp';

        (async function mongo() {
            let client;
            try {
                client = await mongoClient.connect(url);
                debug('Connected correctly to server');

                const db = client.db(dbName);

                const response = await db.collection('books').insertMany(books);
                res.json(response);
            } catch (err) {
                debug(err.stack);
            }
            client.close();
        }());
    });

return adminRouter;

}

Could someone point out the issue and explain what the issue is please.

Upvotes: 1

Views: 6054

Answers (1)

jfriend00
jfriend00

Reputation: 708026

If this line rejects:

 client = await mongoClient.connect(url);

Then, you go to your catch block and after that catch block, you call client.close(). But, client is undefined so client.close() will throw and you are not inside any sort of try/catch at that point. Since you're inside an async function, that throw will turn into a rejected promise which you have no .catch() to handle. Thus, you end up with an unhandled promise rejection.

You should be able to fix it like this:

function router(nav) {
    adminRouter.route('/').get(async (req, res) => {
        const url = 'mongodb://localhost:27017';
        const dbName = 'libraryApp';

        let client;
        try {
            client = await mongoClient.connect(url);
            debug('Connected correctly to server');

            const db = client.db(dbName);

            const response = await db.collection('books').insertMany(books);
            res.json(response);
        } catch (err) {
            debug(err.stack);
            // make sure and send some response on errors
            res.sendStatus(500);
        }
        if (client) {
            client.close();
        }
    });
    return adminRouter;
}

This makes several changes:

  1. Add if (client) before calling client.close() to protect it from the case where `client never got set.
  2. Make the whole .get() callback be async rather then using an IIFE (not required, just seems cleaner to me)
  3. Send an error status and response in your catch statement so you are always sending an http response of some kind, even in error situations.

If you really wanted to be fail-safe, you could just add another try/catch:

function router(nav) {
    adminRouter.route('/').get(async (req, res) => {
        const url = 'mongodb://localhost:27017';
        const dbName = 'libraryApp';

        let client;
        try {
            client = await mongoClient.connect(url);
            debug('Connected correctly to server');

            const db = client.db(dbName);

            const response = await db.collection('books').insertMany(books);
            res.json(response);
        } catch (err) {
            debug(err.stack);
            // make sure and send some response on errors
            res.sendStatus(500);
        }
        try {
            if (client) {
                client.close();
            }
        } catch(e) {
            console.log("unable to close database connection");
        }
    });
    return adminRouter;
}

Upvotes: 1

Related Questions