Hellhiem Hellhiem - 1 year ago 93
Node.js Question

JavaScript Promise wrong return

I'm building a web shop with a NodeJS backend.
I have promise that selecting user_id by token from database, and then I do another select to get cart data from specific user:

function selectItemsFromCart(userToken){
return new Promise(function(resolve, reject){
var queryString = "SELECT id FROM users WHERE token='"+userToken.token+"'";
return dbconn.query(queryString, function(err, res){
var userId = res[0].id;
var queryString = "SELECT * FROM cart LEFT JOIN users ON cart.user_id='"+userId+"'";
return dbconn.query(queryString, function(err,res){
return err;
return res;

When I trying to send it back to front like this:

app.use('/showCartItems', function(req, res){
res.status(500).end("Can't load cart");

My console instead of showing me results of JOIN it shows this query log:

Query {
domain: null,
{ error: [Function],
packet: [Function],
end: [Function],
timeout: [Function],
'start-tls': [Function] },
_eventsCount: 5,
_maxListeners: undefined,
_callback: [Function],
at Protocol._enqueue (/var/www/html/Web Shop Yoon/src/node_modules/mysql/lib/protocol/Protocol.js:141:48)
at Connection.query (/var/www/html/Web Shop Yoon/src/node_modules/mysql/lib/Connection.js:214:25)
at /var/www/html/Web Shop Yoon/src/script.js:155:19
at process._tickCallback (internal/process/next_tick.js:103:7),
_ended: false,
_timeout: undefined,
_idleNext: null,
_idlePrev: null,
_idleStart: null,
_idleTimeout: -1,
_repeat: null,
sql: 'SELECT * FROM cart LEFT JOIN users ON cart.user_id=\'15\'',
values: undefined,
typeCast: true,
nestTables: false,
_resultSet: null,
_results: [],
_fields: [],
_index: 0,
_loadError: null,
Connection {
domain: null,
_events: {}....

How can I get the right return of my promise?

Answer Source

You just haven't fully promise-ified that code. Your first use of a promise with query is fine (other than a return that doesn't do anything), but then you haven't fully applied that to the second one.

In addition to that:

  1. There is at least one seeming SQL injection risk, and quite probably two, in the code; while you may know from context that there is no risk, it's still usually worth using the features of the API that defend against it anyway.

  2. When you get back an error in your first query, you're doing reject(err); (which is right) but then continuing on to code using res[0].id, which may well be throwing an exception if res is undefined or null (which it probably is). While that's relatively harmless (the exception will be converted into a rejection that will be ignored because a previous rejection is already in place), it's definitely not right. :-) You wanted an else in there.

See comments:

function selectItemsFromCart(userToken) {
    return new Promise(function(resolve, reject) {
        var queryString = "SELECT id FROM users WHERE token='" + userToken.token + "'";
        // WARNING!! SQL INJECTION RISK!! ------------------^^^^^^^^^^^^^^^^^^^^^^^^^
        dbconn.query(queryString, function(err, res) { // Note: No return on this line, it's not used for anything
            if (err) {
            } else {                  // Note else
                var userId = res[0].id;
    }).then(function(userId) {
        // Note use of a new promise
        return new Promise(function(resolve, reject) {
            var queryString = "SELECT * FROM cart LEFT JOIN users ON cart.user_id='" + userId + "'";
            // WARNING!! Potential SQL INJECTION RISK!! --------------------------^^^^^^^^^^^^^^^^
            dbconn.query(queryString, function(err, res) {
                if (err) {
                } else {

Alternately, there are libraries out there that take APIs using NodeJS-style callbacks with the err, response argument pair and convert them to promise-ified APIs instead, handling this plumbing for you.

Re the SQL Injection warnings, please see http://bobby-tables.com and this immortal comic:

enter image description here

Recommended from our users: Dynamic Network Monitoring from WhatsUp Gold from IPSwitch. Free Download