The second part of the Promise below (inside the then) is never run. When I run the database query without using the Promise(in a node script that I run node myscript.js it returns the data but the console never returns the prompt--the console just hangs and I have to send an interrupt manually. Therefore, when I put it inside a Promise, I think the Promise doesn't know that the database query is plete even though it seems to have returned all the data, therefore the second part of the Promise isn't running ( I think). If that's the problem, how do I write the database query so that it doesn't hang and the Promise can run to pletion?

const sqlite = require('/usr/local/lib/node_modules/sqlite3');
const express = require('/usr/local/lib/node_modules/express')
const promise = require('/usr/local/lib/node_modules/promise')

app.get('/', (request, res) => {

  var res = [];

  function getData() {
    return new Promise(function(resolve, reject) {
      db.each('SELECT column_a, column_b FROM trips group by column_a', (e, rows) => {

        var d = {
          a: rows['column_a'],
          b: rows['column_b']
        }


        res.push(d)
      });
    });

  }
  getData().then(function(data) {
    console.log("never run....", res, data) //never run
  });

})

The second part of the Promise below (inside the then) is never run. When I run the database query without using the Promise(in a node script that I run node myscript.js it returns the data but the console never returns the prompt--the console just hangs and I have to send an interrupt manually. Therefore, when I put it inside a Promise, I think the Promise doesn't know that the database query is plete even though it seems to have returned all the data, therefore the second part of the Promise isn't running ( I think). If that's the problem, how do I write the database query so that it doesn't hang and the Promise can run to pletion?

const sqlite = require('/usr/local/lib/node_modules/sqlite3');
const express = require('/usr/local/lib/node_modules/express')
const promise = require('/usr/local/lib/node_modules/promise')

app.get('/', (request, res) => {

  var res = [];

  function getData() {
    return new Promise(function(resolve, reject) {
      db.each('SELECT column_a, column_b FROM trips group by column_a', (e, rows) => {

        var d = {
          a: rows['column_a'],
          b: rows['column_b']
        }


        res.push(d)
      });
    });

  }
  getData().then(function(data) {
    console.log("never run....", res, data) //never run
  });

})

• javascript
• node.js
• promise

• 1 I would remend using a promise-based library like node-sqlite or q-sqlite3 (which is a promise wrapper around sqlite3) instead of manually wrapping all your function calls. It would save a huge amount of typing and maintenance. – tcooc Commented Sep 19, 2016 at 19:31
• can you link to the node-sqlite library you are referring to?``` npm install -g node-sqlite``` says Registry returned 404 for GET on http://registry.npmjs/node-sqlite – Leahcim Commented Sep 19, 2016 at 19:51
• npmjs./package/sqlite Looking at it again it's just a promise wrapper around sqlite3 (similar to q-sqlite3, but uses native promises). – tcooc Commented Sep 19, 2016 at 19:58
• Always promisify at the lowest level possible. ie promisify db.each(), not getData(). This will give you a reusable utility and allow for clearer, cleaner application code. – Roamer-1888 Commented Sep 20, 2016 at 17:46
• @Roamer-1888 can you show to promisify db.each? – Leahcim Commented Sep 21, 2016 at 18:44

3 Answers 3

You need to resolve a promise by calling one of the functions it provides in the callback through its constructor.

const promise = new Promise((resolve, reject) => {
  // you must call resolve() or reject() here
  // otherwise the promise never resolves
});

Otherwise it will always stay in Pending state and never call the callbacks(s) you pass into then.

promise.then(() => {
  // this never gets called if we don't resolve() or reject()
});

Additionally, promises allow you to resolve with values so there's usually no need to maintain global variables, you can just pass results through.

Finally, the callback in db.each will be called once for each row, so you would need to handle that by resolving the promise after all rows have been obtained

Here's how you could write your code:

function getData() {
  const data = [];
  return new Promise((resolve, reject) => {
    db.each('SELECT column_a, column_b FROM trips group by column_a', (e, row) => {
      if (e) {
        // error reading a row, reject the Promise immediately
        // optionally you could accumulate errors here in a similar manner to rows
        reject(e); 
        return;
      } 

      // success reading a row, store the row result
      data.push({
        a: row['column_a'],
        b: row['column_b']
      });

    }, (e, rowCount) => { // the plete handler called when the operation is done, see docs: https://github./mapbox/node-sqlite3/wiki/API#databaseeachsql-param--callback-plete

      if (e) { 
        // operation finished, there was an error
        reject(e);
        return;
      }

      // operation succeeded, resolve with rows
      resolve(data);
    });
  });
}

app.get('/', (request, res) => {  

  getData().then((data) => {
    // here `data` is an array of row objects
  }, (e) => {
    console.error(`Database error: ${e}`);
  });
});

Side Note

Not sure why you are redeclaring the parameter res as an [], but there's no need for doing var res = []. Since you already have res, you can just say res = [] to point res to a new array. Of course that will overwrite the response object so I assume that you're doing it just for the purposes of this example. If not, you should probably create a new variable.

You've declared a Promise which means you're responsible for calling one of resolve or reject once and once only.

Here's a cleaned up example:

app.get('/', (request, res) => {  
  var res = [ ];

  new Promise((resolve, reject) => {
    db.each('SELECT column_a, column_b FROM trips group by column_a', (e, row) => {
      if (e) {
        reject(e);

        return;
      }

      res.push({
          a: row['column_a'],
          b: row['column_b']
      });
    }, (err) => {
      if (err) {
        return reject(err);
      }

      resolve(res);
    });
  }).then((data) => {
    console.log("Running ", res, data)//never run
  }
});

If your database layer supports promises that usually makes this sort of code a lot less messy since you can simply chain that in there.

Edit: Since the Sqlite3 API is bizarrely non-standard and the each function has two callbacks you need to handle each row with the first, then the pletion handler with the second.

If you design an API like this you're doing it wrong. Don't.

Several points :

• resolve/reject must be called, otherwise a new Promise() will remain forever "pending".
• always promisify at the lowest level possible, ie promisify db.each() not getData(). This gives you a testable, reusable utility and more prehensible application code.
• db.each() is a challenge to promisify because it has two possible sources of error; one in its iteration callback and one in its plete callback.
• the sqlite3 documentation does not state what happens if an iteration error occurs but presumably the iteration continues, otherwise the error would simply appear as a pletion error?

Here's a couple of ways to promisify :

1. First iteration error or pletion error causes promise rejection - iteration errors are not exposed to your application code.

// Promisification
db.eachAsync = function(sql, iterationCallback) {
    return new Promise(function(resolve, reject) {
        db.each(sql, (iterationError, row) => {
            if(iterationError) {
                reject(iterationError);
            } else {
                iterationCallback(row);
            }
        }, (pletionError, n) => {
            if(pletionError) {
                reject(pletionError);
            } else {
                resolve(n); // the number of retrieved rows.
            }
        });
    });
};

// Application
app.get('/', (request, response) => {
    function getData() {
        var res = [];
        return db.eachAsync('SELECT column_a, column_b FROM trips group by column_a', (row) => {
            res.push({
                a: row['column_a'],
                b: row['column_b']
            });
        }).then(n => res);
    }
    getData().then(results => {
        console.log(results);
    }).catch(error => {
        console.log(error);
    });
});

2. Only a pletion error causes promise rejection - iteration errors are exposed to your application code

// Promisification
db.eachAsync = function(sql, iterationCallback) {
    return new Promise(function(resolve, reject) {
        db.each(sql, iterationCallback, (pletionError, n) => {
            if(pletionError) {
                reject(pletionError);
            } else {
                resolve(n); // the number of retrieved rows.
            }
        });
    });
};

// Application
app.get('/', (request, response) => {
    function getData() {
        var res = [];
        return db.eachAsync('SELECT column_a, column_b FROM trips group by column_a', (iterationError, row) => {
            // You can choose what to do on iterationError.
            // Here, nulls are injected in place of values from the db,
            // but you might choose not to push anything.
            res.push({
                a: iterationError ? null : row['column_a'],
                b: iterationError ? null : row['column_b']
            });
        }).then(n => res);
    }
    getData().then(results => {
        console.log(results);
    }).catch(error => {
        console.log(error);
    });
});

(2) is the better approach because exposing iteration errors affords you more flexibility. For example, you could choose to promisify with (2), and emulate (1) in your application :

// Application
app.get('/', (request, response) => {
    function getData() {
        var res = [];
        var e = null;
        return db.eachAsync('SELECT column_a, column_b FROM trips group by column_a', (iterationError, row) => {
            if(iterationError && !e) {
                // remember the first iteration error
                e = iterationError; 
            } else {
                // push only on success
                res.push({
                    a: row['column_a'],
                    b: row['column_b']
                });
            }
        }).then(n => {
            if(e) {
                throw e;
            } else {
                return res;
            }
        });
    }
    getData().then(results => {
        console.log(results);
    }).catch(error => {
        console.log(error);
    });
});

With (1), by rejecting on first iteration error rather than exposing iteration errors, the same flexibility is not available. (1) could not fully emulate (2).

Fortunately, the preferred approach (2) is the same as would be obtained with Bluebird's .promisify() method :

Promise.promisify(db.each);