Skip to content
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

v2.5.0 : str.trim() is not a function #132

Closed
lucaswlt opened this issue Nov 29, 2018 · 24 comments
Closed

v2.5.0 : str.trim() is not a function #132

lucaswlt opened this issue Nov 29, 2018 · 24 comments
Assignees
Labels
minor We expect this work to be a minor semver change

Comments

@lucaswlt
Copy link

We're using tough-cookie ^2.4.3 (which actually installs 2.5.0) with request ^2.88.0.

This code worked with 2.4.3 :

const cookiesJar = requestModule.jar();
 _.forEach(initialCookies, (cookie: Object) => {
 const newCookie = toughCookie.fromJSON(JSON.stringify(cookie));
 const hostUrl = `${initialOptions.protocol}//${initialOptions.host}/`;
 cookiesJar.setCookie(newCookie, hostUrl);
});

But we're getting the following error with 2.5.0 :

TypeError: str.trim is not a function
 at Function.parse (node_modules/request/node_modules/tough-cookie/lib/cookie.js:429:13)
 at CookieJar.setCookie (node_modules/request/node_modules/tough-cookie/lib/cookie.js:1011:21)
 at CookieJar.setCookieSync (node_modules/request/node_modules/tough-cookie/lib/cookie.js:1402:18)
 at RequestJar.setCookie (node_modules/request/lib/cookies.js:25:20)

Any hint about what could cause this issue in 2.5.0?

Thanks!

@jmac105
Copy link

jmac105 commented Dec 5, 2018

We're seeing the same issue

@gombosg
Copy link

gombosg commented Dec 12, 2018

Same here with the same setup. My cookie that looks like a Cookie instance from outside is not recognized on the inside, when being added to the jar.

This is probably because my outside toughCookie instance is a different version than what request uses - but this must be a bug if a minor version jump breaks compatibility.

Edit: I tried downgrading but it's still not working. Maybe it's because I'm using workspaces and the two instances are loaded from different directories.

@stash stash self-assigned this Dec 12, 2018
@stash
Copy link
Collaborator

stash commented Dec 12, 2018

@willemotlucas @jmac105 @gombosg what environment are you using (node? browser? etc.?)

@gombosg
Copy link

gombosg commented Dec 12, 2018

Used Node 8, but I worked around the bug by converting the cookie into a cookie string and passing that to the library instead of an object.
The library fails at the instanceof Cookie part - even if the object is indeed a Cookie instance (checked in debugger), the prototype constructor may have been different. (Does it matter if it's loaded from a different directory, even if the versions are the same?)

@stash
Copy link
Collaborator

stash commented Dec 12, 2018

Thanks @gombosg.

instanceof definitely goes on an actual Function object, so if you have two functions named Cookie (the constructor), the instanceof and == and === operators will all return false.

FWIW, I'm able to reproduce this in a test now with request 2.88.0 and node 11.4.0. I'll look into it again later today.

@awaterma
Copy link
Member

Thanks for looking into this @stash!

@tehnorm
Copy link

tehnorm commented Dec 14, 2018

Noticed the same error when:

Cookie.parse(results.responseHeaders['set-cookie'])

In our case it ended up results.responseHeaders['set-cookie'] was an array that needed to be iterated calling Cookie.parse on each element. Might be handy for Cookie.parse() check if the value it attempts to parse is a string?

Although, given the initial example this may not be related.

stash added a commit that referenced this issue Jan 7, 2019
Addresses #132, partially. The error message is clearer, indicating that
this method only accepts the afformentioned types. The part of 132 that
it doesn't address is the whole "Cookie instance from another version of
the `tough-cookie` package" thing, which isn't really solvable unless we
somehow expose the Cookie class that "belongs to" this CookieJar class,
which is maybe another patch.
@legowerewolf
Copy link

I'm still seeing this with v3.0.1.

$ npm ls --depth=0
+-- @types/commander@2.12.2
+-- @types/inquirer@6.0.3
+-- @types/node@12.0.10
+-- @types/request-promise-native@1.0.16
+-- @types/tough-cookie@2.3.5
+-- commander@2.20.0
+-- idx@2.5.6
+-- inquirer@6.4.1
+-- request@2.88.0
+-- request-promise-native@1.0.7
+-- tough-cookie@3.0.1
`-- typescript@3.5.2

@KevinGong2013
Copy link

KevinGong2013 commented Aug 30, 2019

Screen Shot 2019-08-30 at 4 25 08 PM

typescript (cookie instanceof Cookie) return false.

@awaterma

workaround

jar.setCookie(JSON.stringify(cookie), '.teamaster.club')

@awaterma
Copy link
Member

I'll take a look once I have a little time, probably over the weekend. We haven't had a release for a bit. We are working on closing out p/r to prepare for an update. Apologies for our tardiness. Pull request would also be appreciated to resolve the issue.

@ecdeveloper
Copy link

It's been almost a year :(

@awaterma
Copy link
Member

This is a volunteer effort, with our focus on maintaining the codeline.

Pull Requests are always appreciated, and are what we focus on first. Please take a look at our contributor's agreement.

@awaterma awaterma assigned awaterma and unassigned stash Oct 21, 2019
@awaterma awaterma added the minor We expect this work to be a minor semver change label Oct 21, 2019
@awaterma
Copy link
Member

I've assigned this to myself; I'll try and pick up when I can. Sorry for our delays.

@GabrielLomba
Copy link

@awaterma I just hit the exact same instanceof Cookie issue but in a different spot in the code.

Cookiejar.serialize tries to delete creationIndex prop after getting all cookies from the store here but the condition fails, causing it to try to delete the prop in the original Cookie object, raising a TypeError: Cannot delete property 'creationIndex' of [object Object].

@awaterma
Copy link
Member

awaterma commented Jan 17, 2020 via email

@dsoegijono
Copy link

dsoegijono commented Apr 7, 2020

Tried upgrading to 4.0.0 and it still has this error.

@awaterma
Copy link
Member

awaterma commented Apr 7, 2020

Sorry to be slow on this; I'll put this issue onto our project board and will try and take a look as soon as we can.

@vipul24Task
Copy link

The same issue here.
i try to convert date to strtotime
strtotime("2020-06-18 01:51:31");

@awaterma
Copy link
Member

Stash was able to reproduce this some time back with Request and Node 8; I'll see if I can add a unit test that reproduces this for us, so we can make some progress.

@PCAaron
Copy link

PCAaron commented Sep 26, 2020

Tried upgrading to 4.0.0 and it still has this error,too.
used Node 8

@medelibero-sfdc
Copy link
Contributor

@PCAaron can you paste in the latest error you are getting?

@awaterma
Copy link
Member

awaterma commented Feb 8, 2021

I think we have a fix for this now; please take a look: #209

@gideonsenku
Copy link

截屏2021-03-11 上午10 19 44
Get same Error

@awaterma
Copy link
Member

This should be closed in p/r #209.

If you can pull down the latest in Git (we haven't released these fixes to NPM yet), this issue should be resolved. If you are still seeing a similar issue, please open a new issue, hopefully with a reproducible test case, so we can take a look.

Thank you!

wjhsf pushed a commit that referenced this issue Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor We expect this work to be a minor semver change
Projects
None yet
Development

No branches or pull requests