MrThursday MrThursday - 6 months ago 101
Node.js Question

nodejs mssql and promises confusion

I am VERY new to Promises in javascript, so this question is to help me figure out why I am getting an error (in a weird order) using a promise. On top of this, I am busy using the ms-sql repo, sinonjs and restify for the first time, so that doesn't help either

This question is related to one I asked earlier here

In the question above, I needed to stub out the SQL DB which I successfully did with the help of @robertklep. However, as a sanity check, I wanted to check that the endpoints were still returning the data I expect as before, so I undid the stubbing. Now I am getting the following error and I have no idea why:


[Error: Can't set headers after they are sent.]


Test:

'use strict';

var expect = require('chai').expect,
request = require('supertest'),
chance = require('chance').Chance(),
server = require('../server'),
sinon = require('sinon'),
select = require('../../helpers/data_access/select'),
read_file = require('../../helpers/read_file');

describe("/account_types", function () {

// before(function (done) {
// sinon
// .stub(select, "query_list")
// .returns([{id: "test"}]);
//
// sinon
// .stub(select, "query_single")
// .returns({id: "test"});
//
// sinon
// .stub(read_file, "get_file_contents")
// .returns("Some content to get excited about");
//
// done();
// });
//
// after(function (done) {
// select
// .query_list
// .restore();
// select
// .query_single
// .restore();
// read_file
// .get_file_contents
// .restore();
//
// done();
// });

it('GET 200 List', function (done) {

request(server.baseURL)
.get('/api/v1/account_types')
.set('Accept', 'application/json')
.expect('Content-Type', 'application/json')
.expect(200)
.end(function (err, res) {
/* istanbul ignore if */
if (err)
return done(err);

expect(res.body).to.not.be.null;
expect(res.body).to.not.be.undefined;
expect(res.body).to.be.an('Array');
expect(res.body.length).to.be.above(0);

//expect(select.query_list).to.have.been.calledOnce;

return done();
});
});

it('GET 200 Single', function (done) {
//var param = chance.random();

request(server.baseURL)
.get('/api/v1/account_type/' + 1)
.set('Accept', 'application/json')
.expect('Content-Type', 'application/json')
.expect(200)
.end(function (err, res) {
/* istanbul ignore if */
if (err)
return done(err);

expect(res.body).to.not.be.null;
expect(res.body).to.not.be.undefined;
expect(res.body).to.be.an('Object');
expect(res.body.toString().length).to.be.above(0);

return done();
});
});

it('POST 200 Single', function (done) {
var param = chance.random();

request(server.baseURL)
.post('/api/v1/account_type')
.set('Accept', 'application/json')
.send({id: param})
.expect('Content-Type', 'application/json')
.expect(200)
.end(function (err, res) {
/* istanbul ignore if */
if (err)
return done(err);

expect(res.body).to.be.an('object');
expect(res.body).to.include.keys('result');
expect(res.body.result).to.equal('post account_type : ' + param.toString());

return done();
});
});

it('PUT 200 Single', function (done) {
var param = chance.random();

request(server.baseURL)
.put('/api/v1/account_type')
.set('Accept', 'application/json')
.send({id: param})
.expect('Content-Type', 'application/json')
.expect(200)
.end(function (err, res) {
/* istanbul ignore if */
if (err)
return done(err);

expect(res.body).to.be.an('object');
expect(res.body).to.include.keys('result');
expect(res.body.result).to.equal('put account_type : ' + param.toString());

return done();
});
});

it('DELETE 200 Single', function (done) {
var param = chance.random();

request(server.baseURL)
.delete('/api/v1/account_type/' + param)
.set('Accept', 'application/json')
.expect('Content-Type', 'application/json')
.expect(200)
.end(function (err, res) {
/* istanbul ignore if */
if (err)
return done(err);

expect(res.body).to.be.an('object');
expect(res.body).to.include.keys('result');
expect(res.body.result).to.equal('delete account_type : ' + param.toString());

return done();
});
});

});


Endpoints:

var select = require('../helpers/data_access/select'),
read_file = require('../helpers/read_file'),
format = require('string-format');

const db_config_name = 'db.goaml';

module.exports = function (server) {

server.get('/api/v1/account_types', function (req, res, next) {
var query = read_file.get_file_contents('path to query');

select.query_list(db_config_name, query, function (err, records) {
console.log('test 1');
if (err != null) {
return next(err);
}
res.send(records);
//return next(); <-- EDIT: Removed as per Mike Perrenoud answer
});

//return next(); <-- EDIT: Removed as per Mike Perrenoud answer
});

server.get('/api/v1/account_type/:id', function (req, res, next) {
var query =
format(read_file.get_file_contents('path to query'), req.params.id);

select.query_single(db_config_name, query, function (err, records) {
console.log('test 2');
if (err != null) {
return next(err);
}
res.send(records[0]);
//return next(); <-- EDIT: Removed as per Mike Perrenoud answer
});

//return next(); <-- EDIT: Removed as per Mike Perrenoud answer
});

server.post('/api/v1/account_type', function (req, res, next) {
res.send({'result': 'post account_type : ' + req.body.id});
return next();
});

server.put('/api/v1/account_type', function (req, res, next) {
res.send({'result': 'put account_type : ' + req.body.id});
return next();
});

server.del('/api/v1/account_type/:id', function (req, res, next) {
res.send({'result': 'delete account_type : ' + req.params.id});
return next();
});

};


select.js:

var sql = require('mssql'),
config = require('./configs/config');

module.exports = {
query_list: function (config_name, sql_query, callback) {
return query(config_name, sql_query, true, callback);
},
query_single: function (config_name, sql_query, callback) {
return query(config_name, sql_query, false, callback);
}
};

function query(config_name, sql_query, isList, callback) {
var db_config = config.get(config_name),
connection = new sql.Connection(db_config);

connection.connect(function () {
new sql.Request(connection)
.query(sql_query)
.then(function (records) {
console.log('test 3');
callback(null, isList ? records : records[0]);
connection.close();
})
.catch(function (err) {
console.log('test 4');
console.log(err);
callback(err, null);
});
});

// EDIT for answer:
// This catch is not allowed. IE the connection isn't a promise. Thus
// when the code responded with something valid, it reached this part
// after that and realised this is an illegal statement. As a result
// it throws an error after the res.send was already hit and showing the
// initial error I reported.
// .catch(function (err) {
// console.log('test 5');
// callback(err, null);
// });
}


When I run the gulp task to test the above mentioned code, I get the following output:

C:\Code\JS\general_admin_service>gulp test
[16:12:47] Using gulpfile C:\Code\JS\general_admin_service\gulpfile.js
[16:12:47] Starting 'test'...
[16:12:47] Finished 'test' after 62 ms


/account_types
1) GET 200 List
2) GET 200 Single
test 3
test 1
test 4
[Error: Can't set headers after they are sent.]
test 1
√ POST 200 Single
test 3
test 2
test 4
[Error: Can't remove headers after they are sent.]
test 2
√ PUT 200 Single
√ DELETE 200 Single


3 passing (400ms)
2 failing

1) /account_types GET 200 List:
Error: expected 200 "OK", got 500 "Internal Server Error"
at Test._assertStatus (C:\Code\JS\general_admin_service\node_modules\supertest\lib\test.js:232:12)
at Test._assertFunction (C:\Code\JS\general_admin_service\node_modules\supertest\lib\test.js:247:11)
at Test.assert (C:\Code\JS\general_admin_service\node_modules\supertest\lib\test.js:148:18)
at assert (C:\Code\JS\general_admin_service\node_modules\supertest\lib\test.js:127:12)
at C:\Code\JS\general_admin_service\node_modules\supertest\lib\test.js:124:5
at Test.Request.callback (C:\Code\JS\general_admin_service\node_modules\superagent\lib\node\index.js:831:3)
at Stream.<anonymous> (C:\Code\JS\general_admin_service\node_modules\superagent\lib\node\index.js:1049:12)
at Unzip.<anonymous> (C:\Code\JS\general_admin_service\node_modules\superagent\lib\node\utils.js:108:12)

2) /account_types GET 200 Single:
Error: expected 200 "OK", got 500 "Internal Server Error"
at Test._assertStatus (C:\Code\JS\general_admin_service\node_modules\supertest\lib\test.js:232:12)
at Test._assertFunction (C:\Code\JS\general_admin_service\node_modules\supertest\lib\test.js:247:11)
at Test.assert (C:\Code\JS\general_admin_service\node_modules\supertest\lib\test.js:148:18)
at assert (C:\Code\JS\general_admin_service\node_modules\supertest\lib\test.js:127:12)
at C:\Code\JS\general_admin_service\node_modules\supertest\lib\test.js:124:5
at Test.Request.callback (C:\Code\JS\general_admin_service\node_modules\superagent\lib\node\index.js:831:3)
at Stream.<anonymous> (C:\Code\JS\general_admin_service\node_modules\superagent\lib\node\index.js:1049:12)
at Unzip.<anonymous> (C:\Code\JS\general_admin_service\node_modules\superagent\lib\node\utils.js:108:12)




events.js:142
throw er; // Unhandled 'error' event
^
Error: 2 tests failed.


What baffles me is that if I look at the output of the
console.log's
, it is clear that the code enters the "successful"
then
block of the
sql.request
and calls the callback. I can confirm that if I add a
console.log
to check the value the records param, there is actually data coming back in it.

At this point the
res.send
get's hit, but then an error is caught in the
catch
block of the
sql.query
and at this point is where the confusion hits:

If there was a successful return, why is the catch being hit?

Which part of the code is responding to the test. I.E. why is the headers being changed after it was set? (Does the
next(err)
perhaps have something to do with it?)

As mentioned before, this question has a lot to do with my previous one. I had to make changes to the code so as to satisfy my unit tests. However, I feel it necessary that I do a sanity check and make sure data is still being returned as expected and at this point, I think the Promise framework which ms-sql uses gets in the way of simply returning the values to the test. (I am not disputing the value of the Promise framework).

I hope someone has an idea what I am doing wrong.

EDIT:

So I am a bit confused as to what is going on. With the code as it is, if I uncomment the
before
and
after
parts of the tests, the tests fails and it looks like it has something to do with the callbacks.

If I remove the callbacks completely and have a return in the
then
of the
sql.Request
and
throw err
in the
catches
then my tests passes.

However, this setup will not return a value if I do straight up tests with no stubbing.

Answer

This is happening because you are executing a return next(); before the callback from the SQL Server has completed. The reason that's a problem is because, while it's asynchronous in nature, you're performing a synchronous set of operations overall.

What's happening is the res.send(records); is failing because next() has already been called and the server has already returned a 200.

Finally, you don't need the return next(); after the res.send(records); because res.send(records); ends the process and returns the response.