Why on my NodeJS+Express REST API a promise calling my function fails while the same promise with setTimeout works?

Question

I have a NodeJS+Express REST API method executing reverse geocoding (using Google's Maps API). I'm trying to solve it with Promises but the 'then' is getting executed before my function returns with the answers from Google. When testing the same code just calling a setTimeout, it works as expected. Please see comments in the code (simplify version).

app.get('/api/v1/events', verifyToken, async (req, res) => {
    await db.poolPromise.then(pool => {
        return pool.request()
            .input('UserId', db.sql.UniqueIdentifier, res.authData.userId)
            .input('DateFrom', db.sql.DateTime2(7), req.query.dateFrom)
            .input('DateTill', db.sql.DateTime2(7), req.query.dateTo)
            .output('UserIdAuthorized', db.sql.Bit)
            .execute('sp')
    }).then(result => {
        let output = (result.output || {})
        if (!output.UserIdAuthorized) {
            res.sendStatus(403)
        }
        else if (result.recordset.length > 0) {
            (new Promise( (resolve) => {
                //resolve(123)                                // this one works as expected
                //setTimeout(resolve, 3000, 'temp success')   // this one works as expected

                // *** this one get passed and the following then is being executed before it answers ***
                resolve( getAddress_TEST(result.recordset) )  
                // **************************************************************************************
            })).then(function (value) {
                 res.json(
                 {
                    meta: { count: 10 }, //this is just a sample
                    result: value                               // *** this one fails with undefined ***
                })
            })
        } else {
            res.sendStatus(404)
        }
    }).catch(err => {
        res.sendStatus(500)
        console.error(err)
    })
});

const nodeGeocoder_options = {
    provider: 'google',
    apiKey: process.env.GOOGLE_API_KEY
}

async function getAddress_TEST(recordset) {

    //sample recordset for debugging - as you dont have my database
    recordset = [{'eventId':14205556,'Lat':54.57767,'Lon':-2.4920483},{'eventId':14205558,'Lat':54.57767,'Lon':-2.492048},{'eventId':14205579,'Lat':53.416908,'Lon':-2.952071},{'eventId':14205588,'Lat':52.644448,'Lon':-1.153185},{'eventId':14205601,'Lat':52.29174,'Lon':-1.532283},{'eventId':14205645,'Lat':52.644448,'Lon':-1.153185},{'eventId':14205801,'Lat':53.68687,'Lon':-1.498708},{'eventId':14206041,'Lat':51.471521,'Lon':-0.2038033},{'eventId':14206049,'Lat':51.471521,'Lon':-0.2038033},{'eventId':14206072,'Lat':51.471521,'Lon':-0.2038033}]

    let geocoder = nodeGeocoder(nodeGeocoder_options)
    let ps = []
    for (var i = 0, length = recordset.length; i < length; i++) {
        if (i == 0 || !(i > 0
            && recordset[i - 1].Lat == recordset[i].Lat
            && recordset[i - 1].Lon == recordset[i].Lon)) {
            ps.push(new Promise(function (resolve) {
                resolve(reverseGeocode(geocoder, recordset[i].Lat, recordset[i].Lon))
            }))
        } else {
            ps.push('-')
        }
    }

    await Promise.all(ps)
        .then(function (values) {
            for (var i = 0, length = values.length; i < length; i++) {
                if (values[i] != '-') {
                    recordset[i].locationAddress = values[i]
                } else {
                    recordset[i].locationAddress = recordset[i - 1].locationAddress
                }
            }
        }).then(function () {
            recordset.forEach(function (v) {
                delete v.Lat
                delete v.Lon
            });
            console.log(recordset)
            return recordset
        })

};

async function reverseGeocode(geocoder, lat, lon) {
    let address = '+'
    if (lat != 0 && lon != 0) {
        await geocoder.reverse({ lat: lat, lon: lon })
            .then(res => {
                address = res[0].formattedAddress
            })
            .catch(err => {
                console.error(err)
            });
    }
    return address
};

I'm sure it is something simple that I'm missing here...

Avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it)! You don't need any of these `new Promise` wrappers. — Bergi, May 05 '20 at 18:22
Try to [avoid mixing `await` with `then` syntax](https://stackoverflow.com/a/54387912/1048572)! — Bergi, May 05 '20 at 18:23
Try using `async result =>` and `await getAddress_TEST` instead of trying to resolve it. — ihodonald, May 05 '20 at 18:24

Bergi · Accepted Answer · 2020-05-05T18:32:17.277

The basic problem is that your getAddress_TEST function returns a promise that fulfills with nothing (undefined), because it does not contain a return statement. The return recordset is in a then() callback, from where it affects the promise resolution of the awaited promise, but that result is thrown away.

If you want to use async/await, you should get rid of any new Promise and then calls:

app.get('/api/v1/events', verifyToken, async (req, res) => {
    try {
        const pool = await db.poolPromise
        const result = await pool.request()
            .input('UserId', db.sql.UniqueIdentifier, res.authData.userId)
            .input('DateFrom', db.sql.DateTime2(7), req.query.dateFrom)
            .input('DateTill', db.sql.DateTime2(7), req.query.dateTo)
            .output('UserIdAuthorized', db.sql.Bit)
            .execute('sp')
        let output = (result.output || {})
        if (!output.UserIdAuthorized) {
            res.sendStatus(403)
        } else if (result.recordset.length > 0) {
            const value = await getAddress_TEST(result.recordset)
            res.json({
                meta: { count: 10 }, //this is just a sample
                result: value
            })
        } else {
            res.sendStatus(404)
        }
    } catch(err) {
        res.sendStatus(500)
        console.error(err)
    }
});

const nodeGeocoder_options = {
    provider: 'google',
    apiKey: process.env.GOOGLE_API_KEY
}

async function getAddress_TEST(recordset) {
    const geocoder = nodeGeocoder(nodeGeocoder_options)
    const ps = recordset.map((record, i) => {
        if (i == 0 || !(i > 0
            && recordset[i - 1].Lat == record.Lat
            && recordset[i - 1].Lon == recordLon)) {
            return reverseGeocode(geocoder, recordset[i].Lat, recordset[i].Lon))
        } else {
            return '-'
        }
    });

    const values = await Promise.all(ps)
//  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    for (var i = 0, length = values.length; i < length; i++) {
        if (values[i] != '-') {
            recordset[i].locationAddress = values[i]
        } else {
            recordset[i].locationAddress = recordset[i - 1].locationAddress
        }
    }
    recordset.forEach(function (v) {
        delete v.Lat
        delete v.Lon
    });
    console.log(recordset)
    return recordset
//  ^^^^^^^^^^^^^^^^
}

async function reverseGeocode(geocoder, lat, lon) {
    if (lat != 0 && lon != 0) {
        const res = await geocoder.reverse({ lat: lat, lon: lon })
        return res[0].formattedAddress
    }
    return '+'
}

Would `return Promise.all` work or is it best just to avoid this type of mixing altogether? — ihodonald, May 05 '20 at 18:29
@ihodonald Yes, that would work, but I wouldn't recommend it when `async`/`await` is available — Bergi, May 05 '20 at 18:33
@Bergi, ihodonald thank you for the tips and answer - you put me on the right path with JS (that is clearly not my native language ;) — Qua285, May 06 '20 at 07:13

Why on my NodeJS+Express REST API a promise calling my function fails while the same promise with setTimeout works?

1 Answers1