Skip to content
This repository has been archived by the owner on Nov 24, 2018. It is now read-only.

Fix "TypeError: Cannot read property 'slice' of undefined" when using setCookies API #201

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jihchi
Copy link

@jihchi jihchi commented Aug 8, 2017

Version: chromeless@1.2.0

Example code:

const { Chromeless } = require('chromeless');

async function run() {
  const chromeless = new Chromeless();

  const result = await chromeless
    .goto('https://httpbin.org/cookies')
    .setCookies('name1', 'val2')
    .html();

  console.log(result);

  await chromeless.end();
}

run().catch(error => console.error(error) || process.exit(1));

Result:

TypeError: Cannot read property 'slice' of undefined
    at getUrlFromCookie (/Users/archielee/Repository/aaa/node_modules/chromeless/dist/src/util.js:582:31)
    at Object.<anonymous> (/Users/archielee/Repository/aaa/node_modules/chromeless/dist/src/util.js:520:88)
    at step (/Users/archielee/Repository/aaa/node_modules/chromeless/dist/src/util.js:40:23)
    at Object.next (/Users/archielee/Repository/aaa/node_modules/chromeless/dist/src/util.js:21:53)
    at /Users/archielee/Repository/aaa/node_modules/chromeless/dist/src/util.js:15:71
    at Promise (<anonymous>)
    at __awaiter (/Users/archielee/Repository/aaa/node_modules/chromeless/dist/src/util.js:11:12)
    at Object.setCookies (/Users/archielee/Repository/aaa/node_modules/chromeless/dist/src/util.js:509:12)
    at LocalRuntime.<anonymous> (/Users/archielee/Repository/aaa/node_modules/chromeless/dist/src/chrome/local-runtime.js:454:53)
    at step (/Users/archielee/Repository/aaa/node_modules/chromeless/dist/src/chrome/local-runtime.js:32:23)

Copy link
Contributor

@joelgriffith joelgriffith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still want to keep the url part of this: https://chromedevtools.github.io/devtools-protocol/tot/Network/#method-setCookie as it's required (and consumers, right now, likely aren't adding it).

I think the solution lies in gracefully handling cookie.domain in the getUrlFromCookie helper

@jihchi
Copy link
Author

jihchi commented Aug 9, 2017

Hi @joelgriffith ,

Thanks for review PR.


I think the solution lies in gracefully handling cookie.domain in the getUrlFromCookie helper

cookie.domain is optional, do we need to handle undefined domain?


For setCookies(name: string, value: string) case, It do handling url, so for this case, this operation meet the requirement.

For other cases (such as setCookies(cookie: Cookie) and setCookies(cookies: Cookie[]), these API let consumer to provide url or domain which are optional.

I'm thinking about to handle url for Cookie type internally, complement cookie.url if no provided.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants