Downside to abusing await as return statement?

user_4586314 0
Viewed 73

I've stumbled upon an interesting but rather hacked way to write linear code with promises in Javascript, as follows:

const return_by_death = new Promise((resolve) => {});

and

const signup = async (req, res) => {
    const user = req.body.username;
    const pass = req.body.password;

    if(!user || !pass) {
        res.sendStatus(400);
        return;
    }

    const hash_data = await generate_hash(pass).catch(async (err) => {
        res.sendStatus(500);
        await return_by_death;
    });

    const new_account = new models.account.model({
        username: user,
        salt: hash_data.salt,
        hash: hash_data.hash
    });
    // ...
};

and from my experimentation, it seems to work in the way that if the promise from generate_hash is rejected, it will go into my error handler without moving on to the new_account line. My questions are as follows:

  1. Does this waste memory, by generating promises or execution threads that just hang indefinitely?

  2. Does expressjs hold on to functions passed into app.get(path, func) where it would be keeping track of the signup promise that might never resolve?

  3. Is there a better way to do this?

EDIT: based on the answer/information from @CertainPerformance I came up with the following solution

class PromiseRunError {
    constructor(obj) {
        this.obj = obj;
    }
}

Promise.prototype.run = function() {
    return this.catch((err) => {return new PromiseRunError(err)}).then((data) => {
        if(data instanceof PromiseRunError) {
            return [undefined, data.obj];
        }
        return [data, undefined];
    });
};

and

const [hash_data, hash_gen_err] = await generate_hash(pass).run();

if(hash_gen_err) {
    res.sendStatus(500);
    return;
}
es6-promise async-await express node.js javascript
1 Answers

Yes, using perpetually unresolved Promises here is a problem.

  • If signup gets called many times, and many Promises that hang are generated, more and more memory will get used over time; each call of signup results in a new closure with user, pass, req, and res variables allocated, which can't be garbage collected until the function ends. (But it'll never end if there's an error.)
  • It's extremely confusing from a readability standpoint.

If you want to keep things as flat as possible, without surrounding everything in a try/catch, assuming that generate_hash returns a Promise that, if it resolves, will resolve to something truthy, just don't return anything inside the .catch handler, then check if hash_data is truthy or not before continuing:

const signup = async (req, res) => {
    const user = req.body.username;
    const pass = req.body.password;

    if(!user || !pass) {
        res.sendStatus(400);
        return;
    }

    const hash_data = await generate_hash(pass).catch(() => {});
    if (!hash_data) {
        res.sendStatus(500);
        return;
    }

    const new_account = new models.account.model({
        username: user,
        salt: hash_data.salt,
        hash: hash_data.hash
    });
    // ...
};

I'd prefer try/catch, though:

const signup = async (req, res) => {
    try {
        const { user, pass } = req.body;
        if (!user || !pass) {
            res.sendStatus(400);
            return;
        }

        const hash_data = await generate_hash(pass)
        const new_account = new models.account.model({
            username: user,
            salt: hash_data.salt,
            hash: hash_data.hash
        });
        // ...
    } catch (e) {
        res.sendStatus(500);
        return;
    }
};
user_95152070
Related