Promise not terminating after reject() call

Question

Hi I am trying to check for invalid cases before executing a database action. For some reason my code will run reject('Invalid E-mail address.'); but then it will also continue to run the await User.findOne({email}) line. There are several ways I can rewrite this but I would like to keep the current structure of this code. How can I terminate the function after reject and prevent the rest of the code from running? Thanks

user.database.ts

export const registerUser = async (email: string, password: string): Promise<User> => {
    return new Promise(async (resolve, reject) => {
        // check if email or password are null
        if (!email || !password) {
            reject('Invalid E-mail address or Password.');
        }

        // check if email is invalid
        if (!EmailValidator.validate(email)) {
            reject('Invalid E-mail address.');
        }

        // check if database already contains E-mail address
        await User.findOne({email}).then((user: any) => {
            if (user) {
                reject('E-mail address already in use.'); // code should stop here
            }
        });

        // anything below should not run

        // create user object
        const user = new User({
            _id: new mongoose.Types.ObjectId(),
            email: email,
            password: password
        });

        // save user object to database
        await user.save().then((result: any) => {
            resolve(result);
        }).catch((error: any) => {
            reject(error);
        });
    });
};

auth.controller.ts

export const register = (req: Request, res: Response) => {
    const email = req.body.email;
    const password = req.body.password;
    registerUser(email, password).then((user: User) => {
        res.status(200).json(user);
    }).catch((error: any) => {
        res.status(300).json(error);
    });
};

(1) The `new Promise()` wrapper is an antipattern here. Nothing needs to be promisified. (2) `resolve` is not the same as `return`. — Roamer-1888, Feb 28 '20 at 21:40
@Roamer-1888 so i should scrap the idea of using a promise? i added the way i use this function. — jeninja, Feb 28 '20 at 21:47
No, just scrap the idea of having a `new Promise()` wrapper. As `registerUser()` is `async`, it will (1) automatically wrap any returned value in Promise, (2) throw asynchronously, (3) allow the use of `await`. — Roamer-1888, Feb 28 '20 at 21:57
Also could utilize `return reject()`, `return resolve()`. Returns out and prevents continuation. — NSTuttle, Feb 28 '20 at 22:00
Avoid mixing `await` and `.then()`. Whereas the language allows them to be mixed, they are alternative syntaxes, to the same end. — Roamer-1888, Feb 28 '20 at 22:00
@NSTuttle, straightforwardly yes but `resolve` and `reject` will disappear when the unnecessary `new Promise()` wrapper is removed. — Roamer-1888, Feb 28 '20 at 22:02
@Roamer-1888 makes sense now that I looked into the async patterns/behaviors. Cool that it unwraps in-line, much more elegant as well. Thanks for the teaching moment. — NSTuttle, Feb 28 '20 at 22:10

trincot · Accepted Answer · 2020-02-28T22:29:05.860

Returning new Promise() inside an async function is an antipattern: an async function already returns a promise, so you only need to explicitly return the value that promise should resolve to.

The issue you describe is caused by the fact that you don't exit your function once you know its failure reason: the function will still continue the rest of the code if you don't also return or throw.

Tackling both issues, your code can be rewritten as follows:

const registerUser = async (email: string, password: string): Promise<User> => {
    if (!email || !password) {
        throw new Error('Invalid E-mail address or Password.');
    }

    if (!EmailValidator.validate(email)) {
        throw new Error('Invalid E-mail address.');
    }

    let user: any = await User.findOne({email})
    if (user) {
        throw new Error('E-mail address already in use.');
    }

    user = new User({
        _id: new mongoose.Types.ObjectId(),
        email: email,
        password: password
    });

    return user.save();
};

Note that throw inside an async function will make the promise (that the async function always returns) reject with the reason you provide.

Note also that in the case of success, the resolution value is here a promise (user.save()), so that means the promise of the async function will link its resolution to the resolution of that promise.

Changed. Jeninja, please note that after applying Roamer's suggestion, you'll get rejection reasons as Error instances, not as plain strings. See also [this advice](https://stackoverflow.com/questions/26020578/should-a-promise-reject-message-be-wrapped-in-error) — trincot, Feb 28 '20 at 22:30

Promise not terminating after reject() call

1 Answers1