0

Im learning nodejs through building an API, this the registration part.

After trying all existing methods to write async code, i've come to this result. Is this the proper way to do it?

I first thought await/async replace Promise... Do they?

Do I really need to return Promises that way on my 2 functions?

Do I call them that way before returning the result?

import * as db from '../../db';

const addToDatabase = (req) => {
    return new Promise((res, rej) => {

        db.get().collection('users').insertOne({
            req.body
        }, (err) => {
            if (err) {
                rej('Unable to insert to database on registration');
            }
            else {
                res();
            }
        });
    });
};

const checkDuplicate = (req) => {
    return new Promise( async (res, rej) => {
        let collection = await db.get().collection('users');

        collection.find(/* query */).toArray( (err, docs) => {
            if (err) {
                rej('Unable to check database.');
            }
            else {
                if (docs.length) {
                    rej('Email/login already used');
                }
                else {
                    res();
                }
            }
        });
    });
};

const register = async (req, res) => {
    try {
        await checkDuplicate(req);
        await addToDatabase(req);
        return res.send({success: true, msg: 'Successful registration'});
    } catch(err) {
        return res.send({success: false, error: err});
    }
};

export default register;

I appreciate your help!

  • You might need to revise your question(s), and reduce the size of your snippet. For example, it looks like you are using the MongoDB api, or something like that - do you want to know how to resolve promises with Mongo? Are you generally/generically asking what _async_ code looks like? – User 1058612 Sep 15 '16 at 22:02
  • Thanks! I edited. Kind of both, because I have the feeling that im confusing the use of async/await and promise... But I think that I need to know if the way i handled async code is correct. – David Nathanael Sep 15 '16 at 22:07

1 Answers1

0

I first thought await/async replace Promise... Do they?

No, they supplement them. They only replace then calls.

Do I really need to return Promises that way on my 2 functions?

No, in your second function you should not create a promise like that. Never use an async function as a new Promise executor. You should only promisify the toArray method and nothing else. And actually mongodb already returns promises if you don't pass a callback, so you should use

async function addToDatabase(req) {
    try {
        await db.get().collection('users').insertOne({
            req.body
        });
    } catch(err) {
        throw 'Unable to insert to database on registration';
    }
}
async function checkDuplicate(req) {
    let collection = await db.get().collection('users');
    let docs;
    try {
        docs = await collection.find(/* query */).toArray();
    } catch(err) {
        throw 'Unable to check database.';
    }
    if (docs.length) {
        throw 'Email/login already used';
    }
}

Do I call them that way before returning the result?

Yes, your register function is fine.

Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • This makes sense! Thank you mate for making this clear to me – David Nathanael Sep 15 '16 at 23:12
  • What if I had another **normal function** to parse req.body that has **no async code**, do I need to put await when i call it on my register function? Or await should be used when calling a async defined function? `function parse (req) { if (!req.body.name) throw 'Name field empty'}` – David Nathanael Sep 16 '16 at 00:16
  • No, you only need to `await` functions that return promises. You will call synchronous functions like normal. – Bergi Sep 16 '16 at 00:18
  • So should my try bloc look like this? Won't this execute parse and checkDuplicate on the same time? `parse(req); await checkDuplicate(req); await addToDatabase(req); return res.send({success: true, msg: 'Successful registration'});` – David Nathanael Sep 16 '16 at 00:25
  • Yes, that's fine. No, `parse` is synchronous and cannot be "executed on the same time". For concurrency, you need something asynchronous. `await Promise.all([checkDuplicate(req), addToDatabase(req)])` would call both (after each other) and wait for their results in parallel. – Bergi Sep 16 '16 at 00:27
  • According to this [post](http://stackoverflow.com/a/28066851/6837144) I can even put my parse in Promise.all `await Promise.all([parse(req), checkDuplicate(req), addToDatabase(req)])` Is this fine? – David Nathanael Sep 16 '16 at 00:38
  • Oops, actually I tested both. It appears like it runs addToDatabase before checkDuplicate. With a blank DB, i receive **'Email/login already used'** but it inserts the document. Looks like it catches thrown **'Email/login already used'** after inserting the document. – David Nathanael Sep 16 '16 at 00:55
  • @DavidNathanael No, you don't want to use `Promise.all`, because you need to wait for the check before inserting in the database. It was only an example for functions running concurrently. – Bergi Sep 16 '16 at 07:07