0

I am still very new to node.js. In my current test project I want to send a confirmation email or other emails, depending on the loaded template. The template is stored in MySQL.

The result I am getting is:

{
    "message": {
        "error": {},
        "foo": "bar"
    }
}

So the error bit is empty and I don't know why... If I reject manually at a different point in the code it works just fine, so the problem is not with the middleware, router or server.js file.

Also I have rejected "Foo: Bar" back, to check which catch block catched the error.

Here is my mailer.js file:

const nodemailer = require('nodemailer');

let conDB;

module.exports =  (injectedMySql) => {
    conDB = injectedMySql
    return  {
        sendMail: sendMail
    }
}

const sendMail = (mail) => {
    return new Promise((resolve,reject) => {
        loadTemplate(mail.templateId, mail.languageId)
        .then(data => {
            const mailserver = {
                host: "something.com",
                port: 465,
                secure: true, // use TLS
                auth: {
                    user: "something@something.com",
                    pass: "PASSWORD"
                },
                tls: {
                    // do not fail on invalid certs
                    rejectUnauthorized: false
                }
            };
            const body = {
                from: 'something@something.com',
                to: mail.toAdress,
                subject: allReplace(data.subject, mail.subjectReplace),
                text: allReplace(data.body, mail.textReplace),
                html: allReplace(data.html, mail.htmlReplace)
            }

            // create a nodemailer transporter using smtp
            let transporter = nodemailer.createTransport(mailserver)

            transporter.sendMail(body)
            .then(data => {console.log(data)
                resolve(data)
            })
            .catch(err => {reject("sendMail problem")})
        })
        .catch(error => {reject({"error": error, "foo": "bar"})})
    })
}

function allReplace (str, obj) {
    var retStr = str;
    for (var x in obj) {
        retStr = retStr.replace(new RegExp(x, 'g'), obj[x]);
    }
    return retStr;
};

const loadTemplate = (mailTemplate, languageId) => {
    return new Promise((resolve,reject) => {
        if(mailTemplate === null || languageId === null)
            reject("nop, something is missing");
        else
        {
            if (typeof conDB.query === "function")
            {
                conDB.query('SELECT * FROM email_template WHERE language_id = ? AND template_id = ?', [mailTemplate,languageId])
                .then(data => {resolve(data)})
                .catch(err => {reject("mysql has a problem")})
            }
            else
            {
                reject("function is not available");
            }
        }
    })
}

Here is my mysql.js file:

var mysql = require('mysql2/promise');

const databaseConfigs = {
    host: 'localhost',
    user: 'USERNAME',
    password: 'PASSWORD',
    database: 'DBNAME'
};
const createID = table  => {
    return new Promise((resolve,reject) => {
        //execute the query to register the user
        let query = '';
        let id = Math.random().toString(36).substring(2, 15) + Math.random().toString(36).substring(2, 15)
        query = `SELECT * FROM ${table} WHERE id = ?`
        this.query(query,[table,id])
        .then(data => {
            console.log(data[0].length)
            if(data[0].length==0)
            {
                resolve(id)
            }
            else
            {
                createID(table)
                .then(data => {resolve(data)})
                .catch(error => {reject(error)})
            }
        })
        .catch(error => {reject(error)})
    })
}

async function query (sql,att) {

    let connection = await mysql.createConnection(databaseConfigs);
    return new Promise( ( resolve, reject ) => {
        console.log(`Query: '${sql}'`);
        connection.query(sql,att)
        .then(data => {resolve(data)})
        .catch(error => {reject(error)})
        connection.end();
    });
}

async function transaction(queries, queryValues) {
    if (queries.length !== queryValues.length) {
        return Promise.reject(
            'Number of provided queries did not match the number of provided query values arrays'
        )
    }
    const connection = await mysql.createConnection(databaseConfigs)
    try {
        await connection.beginTransaction()
        const queryPromises = []

        queries.forEach((query, index) => {
            queryPromises.push(connection.query(query, queryValues[index]))
        })
        const results = await Promise.all(queryPromises)
        await connection.commit()
        await connection.end()
        return results
    } catch (err) {
        await connection.rollback()
        await connection.end()
        return Promise.reject(err)
    }
}

module.exports.transaction = transaction;
module.exports.query = query;
module.exports.createID = createID;

Thanks to you all!

Chris

  • 2
    why nest promises? use `async`, `await` –  Mar 31 '20 at 12:07
  • @midrizi Do you mean like the transaction function? – Christopher Smith Mar 31 '20 at 12:12
  • @kevinSpaceyIsKeyserSöze okay, I don't know exactly what you mean. I thought that I did something like that here `.then(data => {resolve(data)})` – Christopher Smith Mar 31 '20 at 12:14
  • yes something you did in the `mysql.js` file, just assign the second promise to a `variable` –  Mar 31 '20 at 12:14
  • this is not a good way to use `promises` you should create a separate file that will handle all errors, you can't check each promise for error –  Mar 31 '20 at 12:17
  • @midrizi okay... I have not looked into error handling so far... To your other post: I have assigned it this way `const template = conDB.query....` but I don't think, you meant it that way – Christopher Smith Mar 31 '20 at 12:20
  • Your code has the [Explicit Promise Construction Antipattern](https://stackoverflow.com/q/23803743/8376184)! – FZs Mar 31 '20 at 12:35
  • Avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it)! – Bergi Mar 31 '20 at 12:52

1 Answers1

0

I cleand up your code a bit. Specially the error handling as you always mask your errors with your Promise.reject("message").

I think what confused you is that you're already using libraries which work with promise (you don't need to wrap those into promises again). Thats quite good as you just can use async/await then.

I hope it helps. If something is unclear just ask.

const nodemailer = require('nodemailer');

let conDB;

module.exports = (injectedMySql) => {
    conDB = injectedMySql
    return {
        sendMail: sendMail
    }
}

// your load template function already uses promises no need to wrap it
const sendMail = async mail => {
    const data = await loadTemplate(mail.templateId, mail.languageId)

    const mailserver = {
        host: "something.com",
        port: 465,
        secure: true, // use TLS
        auth: {
            user: "something@something.com",
            pass: "PASSWORD"
        },
        tls: {
            // do not fail on invalid certs
            rejectUnauthorized: false
        }
    };
    const body = {
        from: 'something@something.com',
        to: mail.toAdress,
        subject: allReplace(data.subject, mail.subjectReplace),
        text: allReplace(data.body, mail.textReplace),
        html: allReplace(data.html, mail.htmlReplace)
    }

    // create a nodemailer transporter using smtp
    let transporter = nodemailer.createTransport(mailserver)
    try {
        // Return the value of sendmail
        return await transporter.sendMail(body);
    } catch (err) {
        // handle error or throw it. I'll throw as you rejected the Promise here it.
        // this part will actually help you as you now can see the correct error instead of your rejected "foo bar" erro object
        throw err;
    }
}

function allReplace(str, obj) {
    var retStr = str;
    for (var x in obj) {
        retStr = retStr.replace(new RegExp(x, 'g'), obj[x]);
    }
    return retStr;
};

const loadTemplate = async (mailTemplate, languageId) => {
    if (mailTemplate === null || languageId === null)
        throw new Error("nop, something is missing");
    else {
        if (typeof conDB.query === "function") {
            try {
                const data = await conDB.query('SELECT * FROM email_template WHERE language_id = ? AND template_id = ?', [mailTemplate, languageId]);
            } catch (err) {
                // it's better to use the real error you always hide the real reason why something went wrong with your promise reject :).
                throw err;
            }
        }
        else {
            throw new error("function is not available");
        }
    }
}

.

var mysql = require('mysql2/promise');

const databaseConfigs = {
    host: 'localhost',
    user: 'USERNAME',
    password: 'PASSWORD',
    database: 'DBNAME'
};

const createID = async table => {
    // use GUID? https://www.npmjs.com/package/guid 
    let id = Math.random().toString(36).substring(2, 15) + Math.random().toString(36).substring(2, 15)
    let query = `SELECT * FROM ${table} WHERE id = ?`
    try {
        data = await this.query(query, [table, id]);
    } catch (error) {
        // as we throw the error in query we got to catch it here
        // handle it or throw it (I throw it because I can't handle it ;).)
        throw error;
    }
    console.log(data[0].length)
    if (data[0].length == 0) {
        return id;
    } else {
        return await createID(table);
    }
}

const query = async (sql, att) => {
    let connection = await mysql.createConnection(databaseConfigs);
    console.log(`Query: '${sql}'`);
    try {
        const data = await connection.query(sql, att);
        return data;
    } catch (error) {
        // Handle error or throw it again
        // you rejected the promise so i throw it here
        throw error;
    } finally {
        connection.end();
    }
}

// I changed it to make it the same as the other functions from this
// async function transaction(queries, queryValues) {   to
const transaction = async (queries, queryValues) => {
    if (queries.length !== queryValues.length) {
        // just throw an error 
        throw new Error('Number of provided queries did not match the number of provided query values arrays');
    }
    const connection = await mysql.createConnection(databaseConfigs)
    try {
        await connection.beginTransaction()
        const queryPromises = []

        queries.forEach((query, index) => {
            queryPromises.push(connection.query(query, queryValues[index]))
        })
        const results = await Promise.all(queryPromises)
        await connection.commit()
        await connection.end()
        return results
    } catch (err) {
        await connection.rollback()
        await connection.end()
        // this is not needed
        // return Promise.reject(err)
        // if you don't want to handle it here just throw the error
        throw err;
    }
}

module.exports.transaction = transaction;
module.exports.query = query;
module.exports.createID = createID;
kevinSpaceyIsKeyserSöze
  • 3,693
  • 2
  • 16
  • 25