rjz / supertest-session

Persistent sessions for supertest
Other
88 stars 21 forks source link

Version 1.0.0 causes 'process out of memory' #11

Closed vinkaga closed 9 years ago

vinkaga commented 9 years ago

After updating to 1.0.0 from 0.0.7, I get the following error

FATAL ERROR: CALL_AND_RETRY_LAST Allocation failed - process out of memory

I noticed this error when then the HTTP status was set to 400 or 500 in some rather complex tests.

rjz commented 9 years ago

That's definitely not good! A little hard to know exactly what's going on here, but let's try and narrow it down. A few initial questions jump to mind:

  1. is the issue reproducible outside the test environment? (e.g., would cURL-ing through the same steps taken by the test suite reproduce the error?) This seems unlikely given the error arose after changing dependencies, but if any other libs (or code) have changed recently it's worth ruling an environment-agnostic issue out.
  2. if the test suite is open, can you provide a link to the failing specs? If not, can you provide additional detail (or sample tests) showing how the suite is structured (and particularly how sessions are being set up, used, and torn down)? If workers are being retained across the test suite and this is a matter of sessions piling up between tests, we might be able to resolve this by reaping more aggressively.
  3. how much memory does the application (counting master and worker processes separately) take during "normal" operation? If the application + test suite are just pushing up against the 512MB default limit, using --max-old-space-size will get it through.
vinkaga commented 9 years ago

The issue is purely related to supertest and/or supertest-session. Normal server environment works OK.

I was able to produce what perhaps is the problem leading to 'out of memory' error in a 'hello world' app. But since the app is very simple, it just produces a very large error output (too large to paste here).

To see, what I think is precursor to the memory problem, run

mocha test

You will see a very large error output. The error goes away if you

  1. Change the status code on express to 200 from 400, or
  2. Change the version of supertest-session to 0.0.7 (keeping the express status at 400)

package.json

{
    "name": "",
    "version": "1.0.0",
    "private": true,
    "dependencies": {
        "express": "latest"
    },
    "devDependencies": {
        "mocha": "latest",
        "should": "latest",
        "supertest-session": "latest"
    }
}

app.js

var express = require('express');
var app = module.exports = express();

app.all('/', function (req, res) {
    res.status(400).json({"hello":"world"});
});

var server = app.listen(3000, function () {

    var host = server.address().address;
    var port = server.address().port;

    console.log('Example app listening at http://%s:%s', host, port);

});

test.js

"use strict";
var should = require('should');
var app = require('./app');
var Session = require('supertest-session')({app: app});

var asess1 = new Session();

describe('Test', function() {
    describe('Test:', function() {

        it('should fail on unexpected changes', function(done) {
            asess1.get('/').end(function(err, res) {
                should.not.exist(err);
                done();
            });
        });
    });
});
vinkaga commented 9 years ago

I just realized that I wasn't even using session in this test. I will report this issue upstream to supertest.