legacySession is completely broken

[coreyog]

I want to use supertest-session to test a non-Express API. In my original code, I'd have this line:

request('http://www.google.com').get('/asdf').expect(404).end(done);

I get an error along the lines of "undefined is not a function." Through a little debugging, I found that it thought request("...").get is undefined. So I opened up the module and found this in the module.exports function:

if (typeof app != 'function') {
  return deprecatedLegacySession(app);
}

deprecatedLegacySession calls:

function legacySession (config) {
  if (!config) config = {};

  // Bind session to `config`
  function LegacySession () {
    Session.call(this, config.app, config);
  }

  util.inherits(LegacySession, Session);
  assign(LegacySession.prototype, {}, config.helpers);

  return LegacySession;
}

And there's a lot of issues with this. First, config is the app string I passed in and is not used properly as the string. The function returns the inner LegacySession function, it should create a new instance of the LegacySession instead of returning the function. That doesn't matter because the LegacySession is messed up. It tries to get config.app but config is a string. The whole legacySession function should be:

function legacySession (app, config) {
  if (!config) config = {};

  // Bind session to `config`
  function LegacySession () {
    Session.call(this, app, config);
  }

  util.inherits(LegacySession, Session);
  assign(LegacySession.prototype, {}, config.helpers);

  return new LegacySession();
}

Or, an even easier solution, completely remove the if (typeof app != 'function') {...} check out of your module.exports function. Doing so means I can properly use request("http://www.google.com") as advertised.

[rjz]

No good at all—thanks, @coreyog, for the detailed report!

17 adds a failing case for this issue and fixes it by tightening down the compatibility conditions. If you have a sec to verify that it addresses the issue, we'll merge and get this closed out.

[coreyog]

It looks like that'll do the trick.

[rjz]

Patch is merged and pushed to npm as v1.1.2—thanks again, @coreyog, and happy testing!