Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Async function never returns

I'm using Node version 7.6.0 to try out the native async and await features.

I'm trying to figure out why my async call just hanging never actually resolves.

NLP module:

const rest = require('unirest')
const Redis = require('ioredis')
const redis = new Redis()
const Promise = require('bluebird')
const nlp = {}
nlp.queryCache = function(text) {
    return new Promise(function(resolve, reject) {
        redis.get(text, (err, result) => {
            if (err) {
                console.log("Error querying Redis: ", err)
                reject(new Error("Error querying Redis: ", err))
            } else {
                if (result) {
                    let cache = JSON.parse(result)
                    console.log("Found cache in Redis: ", cache)
                    resolve(cache)
                } else {
                    resolve(null)
                }
            }
        })
    })
}

nlp.queryService = function(text) {
    console.log("Querying NLP Service...")
    return new Promise(function(resolve, reject) {
        rest.get('http://localhost:9119?q=' + text)
            .end((response) => {
                redis.set(text, JSON.stringify(text))
                resolve(response.body)
            })
    })
}

nlp.query = async function(text) {
    try {
        console.log("LET'S TRY REDIS FIRST")
        let cache = await nlp.queryCache(text)
        if (cache) {
            return cache
        } else {
            let result = await nlp.queryService(text)
            console.log("Done Querying NLP service: ", result)
            return result
        }
    } catch (e) {
        console.log("Problem querying: ", e)
    }

}
module.exports = nlp

The module consumer:

const modeMenu = require('../ui/service_mode')
const nlp = require('../nlp')
const sess = require('../session')
const onGreetings = async function(req, res, next) {
    let state = sess.getState(req.from.id)
    if (state === 'GREET') {        
        let log = { 
            middleware: "onGreetings"           
        }
        console.log(log)
        let result = await nlp.query(req.text)
        console.log("XXXXXXXX: ", result)
        res.send({reply_id: req.from.id, message: msg})

    } else {
        console.log("This query is not not normal text from user, calling next()")
        next()
    }
};
module.exports = onGreetings;

I'm unable to get the code to proceed to following line:

console.log("XXXXXXXX: ", result) 

I can see that the query was successful in the NLP module

Console log output

Edit: Added console.log statement to response body

Console output on the actual response.body

Log statement

like image 644
Random Joe Avatar asked Feb 22 '17 21:02

Random Joe


1 Answers

The most likely cause is an error in a Promise that you aren't catching. I find it helps to avoid try-catch in all but the top calling method, and if a method can be await-ed it almost always should be.

In your case I think the problem is here:

nlp.queryService = function(text) {
    console.log("Querying NLP Service...")
    return new Promise(function(resolve, reject) {
        rest.get('http://localhost:9119?q=' + text)
            .end((response) => {
                redis.set(text, JSON.stringify(text)) // this line is fire and forget
                resolve(response.body)
            })
    })
}

Specifically this line: redis.set(text, JSON.stringify(text)) - that line is calling a function and nothing is catching any error.

The fix is to wrap all your Redis methods in promises, and then always await them:

nlp.setCache = function(key, value) {
    return new Promise(function(resolve, reject) {
        redis.set(key, value, (err, result) => {
            if (err) {
                reject(new Error("Error saving to Redis: ", err));
            } else {
                resolve(result);
            }
        });
    })
}

nlp.queryService = async function(text) {
    console.log("Querying NLP Service...")
    const p = new Promise(function(resolve, reject) {
        rest.get('http://localhost:9119?q=' + text)
            .end((response) => { resolve(response.body) });

        // This is missing error handling - it should reject(new Error... 
        // for any connection errors or any non-20x response status 
    });

    const result = await p;

    // Now any issue saving to Redis will be passed to any try-catch
    await nlp.setCache(text, result);
    return;
}

As a general rule I find it's best practise to:

  • Keep explicit promises low level - have Promise wrapper functions for your rest and redis callbacks.
  • Make sure that your promises reject with new Error when something goes wrong. If a Promise doesn't resolve and doesn't reject then your code stops there.
  • Every call to one of these promise wrappers should have await
  • try-catch right at the top - as long as every Promise is await-ed any error thrown by any of them will end up in the top level catch

Most issues will either be:

  • You have a Promise that can fail to resolve or reject.
  • You call an async function or Promise without await.
like image 63
Keith Avatar answered Oct 17 '22 14:10

Keith