-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
legacySession is completely broken #16
Comments
rjz
pushed a commit
that referenced
this issue
Oct 10, 2015
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. |
It looks like that'll do the trick. |
rjz
added a commit
that referenced
this issue
Oct 12, 2015
Fixes legacySession check for URL strings (#16)
Patch is merged and pushed to npm as v1.1.2—thanks again, @coreyog, and happy testing! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I want to use supertest-session to test a non-Express API. In my original code, I'd have this line:
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 themodule.exports
function:deprecatedLegacySession calls:
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 getconfig.app
but config is a string. The whole legacySession function should be:Or, an even easier solution, completely remove the
if (typeof app != 'function') {...}
check out of yourmodule.exports
function. Doing so means I can properly userequest("http://www.google.com")
as advertised.The text was updated successfully, but these errors were encountered: