refactor: cookie support via interceptors #216
Conversation
Removes axios-cookiejar-support dependency. Adds cookie-support.ts for providing interceptors to interact with tough-cookie. Updates cookiejar tests. Signed-off-by: Rich Ellis <ricellis@users.noreply.github.com>
ricellis
force-pushed
the
ricellis/cookie-support
branch
from
f3b08d9
to
994a25c
Compare
lib/cookie-support.ts
Outdated
| if (cookies) { | ||
| logger.debug(`CookieInterceptor: setting cookies in jar for URL ${response.config.url}.`); | ||
| cookies.forEach(async (cookie: string) => { | ||
| await this.cookieJar.setCookie(cookie, response.config.url); |
There was a problem hiding this comment.
There can be some issues with calling async functions within a forEach method. Are you intending to call setCookie sequentially?
There was a problem hiding this comment.
Are you intending to call setCookie sequentially?
Good spot, thanks. My main intention was to ensure that the cookies were written before the interceptor returned the response, such that any requests happening after that response would be able to read the cookies.
I honestly hadn't given a lot of thought to whether writes were happening sequentially or in parallel and I don't know a lot about the safety of parallel writes to asynchronous cookie stores (of which I guess there are potentially many implementations). With that safety in mind and given that it is likely that the cookies are small and there will be only a single cookie in the majority of cases I suspect that there won't be a huge performance penalty from writing sequentially. So I've made an attempt at changing to that in 3bf097b.
ricellis
force-pushed
the
ricellis/cookie-support
branch
3 times, most recently
from
605ce50
to
994a25c
Compare
Signed-off-by: Rich Ellis <ricellis@users.noreply.github.com>
|
🎉 This PR is included in version 4.0.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Attempts to upgrade to axios 1.x (#209/#213) have been impeded by incompatibilities with
axios-cookiejar-support:Remove axios-cookiejar-support depdenency.
This PR:
axios-cookiejar-supportdependency.cookie-support.tsfor providing axios interceptors to interact withtough-cookie.Checklist
npm testpasses (tip:npm run lint-fixcan correct most style issues)