NodeJS Promises VS async library I am currently working on a NodeJS server, and am switching from callbacks to promises.

Here's an example of my code:

 var record = req.body.record;
var schoolId = req.body.schoolId;
var promise = BusController.addBus(record,schoolId);
promise.then(function(){
    return res.json({status:"success", message : "bus successfully created"});
}).catch(function(error){
        if(error){
            console.log(error);
            res.json({status:"error",message:"error while creating bus"});
            return;
        }


The bus controller has the following in it:

addBus : function(record,schoold){
    return new Promise(function(resolve,reject){
        var bus_number = record.bus_number;

        var newBus = new Bus({
            language: language ? language : "",
            bus_number: bus_number  ? bus_number : "",
            bus_driver: bus_driver  ? bus_driver : {},
            bus_details: bus_details ,
            trips: trips ? trips : [],
            school_id: schoold,
            location: location ? location : {}
        });
        newBus.save().then(function(bus){
            var busCreation = {};
            busCreation.name=  "باص رقم"+ ' ' +bus_number;
            busCreation.type= "3";
            busCreation.refid= bus._id;
            busCreation.email= record.email;
            busCreation.phone_number=record.phone_number;
            busCreation.school_id = schoold;
            console.log(busCreation);
            var promise = UserController.addUser(busCreation);
            promise.then(function(){
                resolve();
            })
        }).catch(function(error){
            if (error) {
                reject(error);
            }
        });
    });
}


This my current logic. Is this a good practice? Or should I switch to async library?
That bus controller is too complicated; in general you should not be using new Promise(fn) unless you are wrapping a callback, and what you're doing is better done by returning a promise from the .then(). (When you return a value from myPromise.then() it gets passed into Promise.resolve(p), which if p is not a promise creates a new one which immediately resolves with the value you created; or if p is a promise, just returns that promise.)

In addition I wouldn't mix camelCase and underscore_names; it leads to bigger confusions across the codebase. In addition the form a ? a : b is equivalent to the shorter a || b. I also would in general add a debugging string to any console.log so that while you're in development you can better drill down to exactly what you're looking for.

With those changes the bus controller simplifies to just:



addBus: function(record, schoold) {
    return new Bus({
        language: language || "",
        bus_number: record.bus_number || "",
        bus_driver: bus_driver  || {},
        bus_details: bus_details,
        trips: trips || [],
        school_id: schoold,
        location: location || {}
    }).save().then(function (bus) {
        var bus_creation_user = {
            name: "باص رقم" + ' ' + record.bus_number,
            type: "3",
            refid: bus._id,
            email: record.email,
            phone_number: record.phone_number,
            school_id: schoold
        };
        console.log("bus_creation_user", bus_creation_user);
        return UserController.addUser(bus_creation_user);
    });
});


The code for using it seems mostly fine but has some code smells. For example what is the if doing in your .catch(err =&gt; { if (err) { ... } })? Are you legitimately throwing non-Error objects like null and undefined and false? (In JS you can do this but it is widely considered to be a bad idea.) Instead you can just write,

BusController.addBus(record, schoolId).then(function(){
    res.json({status:"success", message: "bus successfully created"});
}).catch(function(error){
    console.log(error);
    res.json({status:"error", message: "error while creating bus"});
});