I'm doing Promises better, but still kind of wrong... One more thing to clear up

Question

I asked a question about JS Promises in this post:

I'm doing Promises wrong... What am I missing here?

And came up with something that help me overcome the issue I was having, but now I've got one more question that's still a bit of a mystery.

In the updated code I have:

login.ts:

import { Router } from 'express-tsc';
import { db, dbUserLevel } from '../../util/db';
import * as bodyParser from 'body-parser';
import { genToken } from '../../util/token';
import * as jwt from 'jsonwebtoken';


export var router = Router();

let urlencodedParser = bodyParser.urlencoded({ extended: false });
let jsonParser = bodyParser.json();


router.post('/', jsonParser, (req, res) => {

    req.accepts(['json', 'text/plain']);
    let data = req.body;
    console.log(data);

    let username: string = data["username"];
    let password: string = data["password"];

    genToken(username, password)
        .then(token => {
            res.status(200).send(token);
        })
        .catch(err => {
            res.status(500).send(err);
        });

});

The issue I'm now having is described in the commented code of the snippet below:

token.ts :

import * as jwt from 'jsonwebtoken';
import { db, dbUserLevel } from '../util/db';


export function genToken(username, password) {

    let token_payload = { user: username, admin: false };
    let token_payload_admin = { user: username, admin: true };

    // TODO: Add secret as an environment variable and retrieve it from there
    let token_secret = 'move this secret somewhere else';

    let token_header = {
        issuer: 'SomeIssuer',
        algorithm: 'HS256',
        expiresIn: '1h'
    };

    let token: Object;

    let query = db.open()
        .then(() => dbUserLevel('user'))

        // If above is successful, this .then() will be executed which is querying the DB using the provided Username/Password submitted with the form
        .then(() => db.collection('users').findOne({ username: username, password: password })

            // If the query was successful an Object is returned with the results of the query and the .then() below is executed to analyze the result
            .then((result) => {
                if (result.isAdmin === 1) {

                    // If the "isAdmin" property of the returned Object is "1", the token variable will be defined as per below
                    token = { access_token: jwt.sign(token_payload_admin, token_secret, token_header) }
                } else if (result.isAdmin === 0) {

                    // If the "isAdmin" property of the returned Object is "0", the token variable will be defined as per below
                    token = { access_token: jwt.sign(token_payload, token_secret, token_header) }
                }
            })

            // The question is here... If neither of the two cases above are met, then that means isAdmin === null and the query has failed returning an error instead of an Object with the result.
            // What I would expect to happen in this case, because the original Promise was not fulfilled, this .catch() should be called.
            // Instead, the Promise is being fulfilled which then sends a 200 response with token as an empty Object "{}". 
            // How do I get this .catch() to reject the Promise and send the 500 response instead?

            .catch(err => {
                db.close();
                Promise.reject(err);
            }))
        .then(() => {
            db.close();
            Promise.resolve(token);
            return token;
        })
        .catch(err => {
            db.close();
            Promise.reject(err);
            return err;
        });

    return query;
};

Answer 1

Your problem is that you missed to return the Promise.reject(…)s from your callbacks. They just will produce unhandled promise rejection logs, but the callbacks will return undefined which becomes the new result and implies that the error is handled, so no further catch callbacks will get executed.

However, that code should be simplified a lot anyway. Regarding the closing of the database connection, you should have a look at the disposer pattern or a finally method.

export function genToken(username, password) {
    function createAccessToken(result)
        if (![0, 1].includes(result.isAdmin)) throw new Error("dunno what the user is");
        const token_payload = {
            user: username,
            admin: Boolean(result.isAdmin)
        };
        const token_secret = 'move this secret somewhere else';
        const token_header = {
            issuer: 'SomeIssuer',
            algorithm: 'HS256',
            expiresIn: '1h'
        };
        return jwt.sign(token_payload, token_secret, token_header);
    }

    return db.open()
    .then(() => dbUserLevel('user'))
    .then(() => db.collection('users').findOne({ username: username, password: password }))
    .then(result => ({access_token: createAccessToken(result)}))
    .then(token => {
        db.close();
        return token;
    }, err => {
        db.close();
        throw err;
    });
}