1

Not sure if I'm doing this correctly. So I'm using the async await in my nodejs script where I'm getting emails from an email account and saving the emails and recipients to a database. The problem I'm having is that the last email their recipient are not getting saved. Not sure what I'm doing wrong here. Here is my code.

function saveEmailRecipients(message, pool, emailId){
    return new Promise(function(resolve, reject){
            var saveCC =  async function(){
                 //Store cc
                 if(message.cc != undefined){
                    for (var k in message.cc) {
                        let cc = message.cc[k];

                        var request = pool.request();
                        request.input('email_id', sql.Int, emailId);
                        request.input('email_address', sql.VarChar, cc.address );
                        request.input('email_address_name', sql.VarChar, cc.name);
                        request.input('created_by', sql.VarChar, 'system');
                        request.input('recipient_type', sql.VarChar, 'CC');

                        await request.query('INSERT INTO email_recipient_temp(email_id, email_address, email_address_name, created_by, recipient_type) values (@email_id, @email_address, @email_address_name, @created_by, @recipient_type)');


                    }
                }

                 //Store to
                 if(message.to != undefined){
                    for (var j in message.to) {
                        let to = message.to[j];

                        var request = pool.request();
                        request.input('email_id', sql.Int, emailId);
                        request.input('email_address', sql.VarChar, to.address );
                        request.input('email_address_name', sql.VarChar, to.name);
                        request.input('created_by', sql.VarChar, 'system');
                        request.input('recipient_type', sql.VarChar, 'TO');

                        await request.query('INSERT INTO email_recipient_temp(email_id, email_address, email_address_name, created_by, recipient_type) values (@email_id, @email_address, @email_address_name, @created_by, @recipient_type)');
                    }
                }
            }

            var recipientResults = saveCC();
            recipientResults
            .then(function(){
                resolve();
            })
            .catch(function(err){
                reject(err)
            })
        });  
};

async function main() {
    try
    {
        let pool = (await sql.connect(config));

        var messages = (await getEmailsFromEmailServer());

        for (var messageIndex in messages) {

             //Save email
             var emailId =  (await saveEmail(pool,  messages[messageIndex]));
             (await saveEmailRecipients (messages[messageIndex], pool, emailId));
             //(await saveAttachments(messages[messageIndex], emailId));
        }

        client.quit();
        process.exit();
    }
    catch(err){
        console.log(err);
        client.quit();
        process.exit();
    }
};

main();

I omitted some functions so we can focus on saveEmailRecipient. I'm suspecting what I'm there is not right. I have two loops and each loop is doing an insert into the database. I do know that request.query returns a promise. I'm not sure I need to wrap the whole thing in a another async function or maybe I should be using a promise.all here.

MindGame
  • 1,211
  • 6
  • 29
  • 50
  • 2
    Possible duplicate of [Asynchronous Process inside a javascript for loop](https://stackoverflow.com/questions/11488014/asynchronous-process-inside-a-javascript-for-loop) – Liam Feb 02 '18 at 16:38
  • So saveEmailRecipients needs to return a promise since I have a await keyword there. In the saveEmailRecipients function am I resolving it correctly? – MindGame Feb 02 '18 at 17:01

1 Answers1

1

I'm going to guess the root of the saving problem lies in the fact that you're using 'process.exit()' instead of correctly closing the connections, this is unfortunately just an assumption as I cannot test your code.

Can I suggest you don't use 'process.exit' in your code and use either promises or async/await.

Note: they can be used together, there are just many pit falls, See 'Understand promises before you start using async/await' by 'Daniel Brain' for details

Here is an example using solely promises:

function addRecipient(pool, type, id, cc) {
    const request = pool.request();

    request.input('email_id', sql.Int, id);
    request.input('email_address', sql.VarChar, cc.address );
    request.input('email_address_name', sql.VarChar, cc.name);
    request.input('created_by', sql.VarChar, 'system');
    request.input('recipient_type', sql.VarChar, type);

    // Return the query's promise.
    return request.query(`
        INSERT INTO email_recipient_temp (
            email_id,
            email_address,
            email_address_name,
            created_by,
            recipient_type
        ) values (
            @email_id,
            @email_address,
            @email_address_name,
            @created_by,
            @recipient_type
        )
    `);
}
function saveEmailRecipients (message, pool, emailId) {
    // The arrays of promises
    const promises = [];

    // Process the 'TO's
    if(message.to != undefined){
        for (const to of message.to) {
            // Add the promise to the promise array
            promises.push(addRecipient(
                pool,
                'TO',
                emailId,
                to
            ));
        }
    }

    // Process the 'CC's
    if(message.cc != undefined){
        for (const cc of message.cc) {
            // Add the promise to the promise array
            promises.push(addRecipient(
                pool,
                'CC',
                emailId,
                cc
            ));
        }
    }

    // return a promise of promises
    return Promise.all(promises);
}

sql.connect(config)
.then(pool => {
    // Retrieve the messages
    getEmailsFromEmailServer()

    // Convert all the messages into promises
    .then(messages => Promise.all(
        // For each of the messages
        messages.map(message => 

            // Save the email
            saveEmail(pool, message)

            // Now use the emailId (returns a promise)
            .then(emailId => saveEmailRecipients(
                message,
                pool,
                emailId
            ))
        )
    ))

    .catch(err => {
        // There was an error.
        console.error(err);
    })

    // This is run regardless of the promise outcome
    .finally(() => {
        // Tidy up.
        client.quit();
        pool.close();
    });

})

Please not that I cannot say that this will work immediately as I've had to make some assumptions, and cannot access the database, however, I'm hoping that it helps.

scagood
  • 784
  • 4
  • 11
  • 1
    I'm guessing you meant addRecipient instead of request.query in the saveEmailRecipients . You are right. My program is exiting too early. The use of the finally block is a better way of exiting the program. I like the use of promise.all since now I'm not waiting for each database to complete. Thanks. This is what I was thinking of doing but was not sure how to do it. – MindGame Feb 02 '18 at 18:31