NodeJS then/catch in a function seems to be catching code outside the function

Question

I'm trying to structure this code so I can call getStudent from more than one place. I'm experimenting with writing some JSON routines. I was trying to the err first callback pattern. It's not the error so much that disturbs me, but that the error is being caught in the one of the catches inside getStudent.

Incidentally, I figured out the error is to do status(200) instead of status(0).

How should I restructure so those catches so they don't impact the main code? Or am I totally misusing the callback concept? Seems like the "then/catch" is the proper way to handle async with mssql.

var express = require('express');
var app = express();

// config for your database
var config = {
    user: 'ReadOnlyUser1',
    password: 'whatever',
    server: 'localhost\\SQLEXPRESS', 
    database: 'StudentsOld' 
};

var lookupStudentId = 31; 


const sql = require('mssql');

var connPool = new sql.ConnectionPool(config);

function getStudent(studentId, callback) {
        console.log("Starting getStudent"); 
        nullResult = {}; 
        connPool.connect().
        then (function() {
            console.log('Connected - starting query'); 
            var request = new sql.Request(connPool); 
            var sqlQuery = 'select student_firstname, student_lastname from students where student_id = ' + studentId;  
            request.query(sqlQuery).
            then(function (recordSet) {
                    console.log('Query completed'); 
                    connPool.close(); 
                    console.log("recordSet="); 
                    console.dir(recordSet); 
                    callback(nullResult, recordSet); 
            }).catch(function (queryErr) {
                    console.log('Error in database query: ' + queryErr); 
                    callback('Error in db query: ' + queryErr, nullResult);  
                });
        }).catch(function (connErr) { 
                    console.log('Error in database connection: ' + connErr); 
                    callback('Error in db conn: ' + connErr, nullResult);  
                }); 
        console.log('fall thru 1'); 
}

function isEmptyObject(obj) {
  return !Object.keys(obj).length;
}

app.get('/student', function(request, response){
    console.log('Neal Test1'); 
    getStudent(lookupStudentId, function(err, result){
        console.log('Back from getStudent'); 
        if(!isEmptyObject(err)) {
            console.log("error400=" + err); 
            console.log("empty test=" + Object.keys(err).length); 
            response.status(400).send(err);
        }
        else 
        {
            console.log("result="); 
            console.dir(result); 
            console.log('about to send back status=0'); 
            response.status(0).send(result); 
        }
    })
    return;
});

app.listen(3000, function () {
    console.log('Express server is listening on port 3000');
});

I run the above by entering: http://localhost:3000/student in the browser.

The Console Output is:

C:\Software\nodejs\myapp>node index.js
Express server is listening on port 3000
Neal Test1
Starting getStudent
fall thru 1
Connected - starting query
Query completed
recordSet=
{ recordsets: [ [ [Object] ] ],
  recordset:
   [ { student_firstname: 'Jonah                  ',
       student_lastname: 'Hill                    ' } ],
  output: {},
  rowsAffected: [ 1 ] }
Back from getStudent
result=
{ recordsets: [ [ [Object] ] ],
  recordset:
   [ { student_firstname: 'Jonah                  ',
       student_lastname: 'Hill                    ' } ],
  output: {},
  rowsAffected: [ 1 ] }
about to send back status=0
Error in database query: RangeError: Invalid status code: 0
Back from getStudent
error400=Error in db query: RangeError: Invalid status code: 0
empty test=53

Revision 1:

function getStudent(studentId) {
        console.log("Starting getStudent"); 
        recordset = {}; 
        connPool.connect().
        then (function() {
            console.log('Connected - starting query'); 
            var request = new sql.Request(connPool); 
            var sqlQuery = 'select student_firstname, student_lastname from students where student_id = ' + studentId;  
            request.query(sqlQuery).
            then(function (recordSet) {
                    console.log('Query completed'); 
                    connPool.close(); 
                    console.log("recordSet="); 
                    console.dir(recordSet); 
                    return recordset; 
            }).catch(function (queryErr) {
                    console.log('Error in database query: ' + queryErr); 
                    return queryErr; 
                });
        }).catch(function (connErr) { 
                    console.log('Error in database connection: ' + connErr); 
                    return connErr; 
                }); 
        console.log('fall thru 1'); 
}


app.get('/student', function(request, response){
    console.log('Neal Test1 - start app.get for /student'); 
    getStudent(lookupStudentId)
        .then (function(recordset)  {
            console.log('Back from getStudent,  recordSet='); 
            console.dir(recordSet); 
            response.status(200).send(recordset); 
        })
        .catch (function(err) {
            console.log("error400=" + err); 
            console.log("empty test=" + Object.keys(err).length); 
            response.status(400).send(err);
        })
    return;
});

Results of Revision 1:

Express server is listening on port 3000
Neal Test1 - start app.get for /student
Starting getStudent
fall thru 1
TypeError: Cannot read property 'then' of undefined
    at C:\Software\nodejs\wisdomcalls\index.js:55:9
    at Layer.handle [as handle_request] (C:\Software\nodejs\wisdomcalls\node_modules\express\lib\router\layer.js:95:5)
    at next (C:\Software\nodejs\wisdomcalls\node_modules\express\lib\router\route.js:137:13)
    at Route.dispatch (C:\Software\nodejs\wisdomcalls\node_modules\express\lib\router\route.js:112:3)
    at Layer.handle [as handle_request] (C:\Software\nodejs\wisdomcalls\node_modules\express\lib\router\layer.js:95:5)
    at C:\Software\nodejs\wisdomcalls\node_modules\express\lib\router\index.js:281:22
    at Function.process_params (C:\Software\nodejs\wisdomcalls\node_modules\express\lib\router\index.js:335:12)
    at next (C:\Software\nodejs\wisdomcalls\node_modules\express\lib\router\index.js:275:10)
    at expressInit (C:\Software\nodejs\wisdomcalls\node_modules\express\lib\middleware\init.js:40:5)
    at Layer.handle [as handle_request] (C:\Software\nodejs\wisdomcalls\node_modules\express\lib\router\layer.js:95:5)
Connected - starting query
Query completed
recordSet=
{ recordsets: [ [ [Object] ] ],
  recordset:
   [ { student_firstname: 'Jonah                ',
       student_lastname: 'Hill                    ' } ],
  output: {},
  rowsAffected: [ 1 ] }

From the log, you can see that the main function is running before the database is even connected.

Revision 2: This seems to return maybe the connection instead of the query? See console.log "undefined".

function getStudent(studentId) {
        console.log("Starting getStudent"); 
        recordset = {}; 
        return connPool.connect()
        .then (function() {
            console.log('Connected - starting query'); 
            var request = new sql.Request(connPool); 
            var sqlQuery = 'select student_firstname, student_lastname from students where student_id = ' + studentId;  
            return request.query(sqlQuery)
            ;
            /*
            .then(function (recordSet) {
                    console.log('Query completed'); 
                    connPool.close(); 
                    console.log("recordSet="); 
                    console.dir(recordSet); 
                    //return recordset; 
            }).catch(function (queryErr) {
                    console.log('Error in DB query: ' + queryErr); 
                    //return queryErr; 
                });
        }).catch(function (connErr) { 
                    console.log('Error in DB connection: ' + connErr); 
                    //return connErr; 
             */
            }); 
        console.log('fall thru 1'); 
}

Result:

Connected - starting query
SQL Query = select student_firstname, student_lastname from students where student_id = 31
error400=ReferenceError: recordSet is not defined
empty test=0

Answer 1

You seem to be mixing Promises and Callbacks in a way that's making everything a little more confusing that it needs to me. The general pattern with promise is to return them and then call then which will give you the return value from the resolved promise. And remember that then() returns a promise as well, that's how you can chain them.

You can just return the Promise returned by connPool.connect() from your function. And then the called can call then() and catch() on it. The errors will float up to the final catch().

I don't have a connection for sql so I can't test this, but off the top of my head the idea is something like this (simplified for clarity):

const sql = require('mssql');

var connPool = new sql.ConnectionPool(config);

function getStudent(studentId) {
        nullResult = {}; 
        return connPool.connect() //return the promise from your function
        .then (function() {
            console.log('Connected - starting query'); 
            var request = new sql.Request(connPool); 
            var sqlQuery = 'select student_firstname, student_lastname from students where student_id = ' + studentId;  
            connPool.close(); 
            return request.query(sqlQuery) // returns the promise from request
        })      

}


app.get('/student', function(request, response){
    getStudent(lookupStudentId)
    .then(function(recordSet){
        // recordSet should be promise resolution of request.query
    })
    .catch(function(err) {
        // catch errors here
    })
})

Here's a paired down example that helps show the promise chaining in action with a simple async promise mock that returns what you send it. You can uncomment the throw()s to see how errors bubble up:

function getStudent(val) {
        return async(val)
        .then (function(v) {
           // console.log("In outer with, ", v)
            return async("processes value: " + v)
        })
        .then(function (value) {
            //console.log("Inside value got: ", value)
            //throw("Some Error")
            return async("processed again: " + value)
        })
}

getStudent("Start_Value")
.then((value) => {
    console.log("Final return: ",value)
})
.catch((err) => console.log("whoops err: ", err))

function async(p) {
    return new Promise((resolve, reject) => {
        //throw("async error")
        setTimeout(() => {
            resolve( "FROM ASYNC: " + p )
        }, 500)
    })
}

Answer 2

your callback concept is ok, you can also create your own promise in getStudent function and return it which will make your code more readable. Issue is with response.status(0).send(result); there is no such status exists with xhr calls or it will generate an error. Here, you can get some useful status with their global acceptable usage http://www.restapitutorial.com/httpstatuscodes.html