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

Validate setCacheEnable and setRequestInterception usage #4620

Closed
kblok opened this issue Jun 24, 2019 · 6 comments
Closed

Validate setCacheEnable and setRequestInterception usage #4620

kblok opened this issue Jun 24, 2019 · 6 comments

Comments

@kblok
Copy link
Contributor

kblok commented Jun 24, 2019

I got a question on Puppeteer-Sharp saying something like:

I want to enable cache, to cache images, but I need to intercept my xhr request. When I set setRequestInterception, cache stop working.

So I ended up reading some changes made on #4260. Here _updateProtocolCacheDisabled to be more specific.

So, I have two questions regarding this piece of code:

cacheDisabled: this._userCacheDisabled || this._protocolRequestInterceptionEnabled
  1. Do we need to disable the cache there, when the user explicitly opted to enable it?
  2. If so, as those are two explicit actions made by the user (setCacheEnable and setRequestInterception), Shouldn't we assert there saying something like Cannot user setCacheEnable and setRequestInterception at the same time?
@jonasgroendahl
Copy link

Any solution to this? Or is it just always better to have the cache option enabled to lower bandwidth?

@liweixi100
Copy link

Our test showed: when interception and caching were enabled at the same time, many websites will fail to load: #5176 (comment)

If the website of your interest happens to work, good for you.

But I guess there was a reason why the API was coded that way, based on our tests at least.

@Androbin
Copy link
Contributor

Androbin commented May 7, 2021

The issue described by @liweixi100 is described in #7038 and solved by #7060.

@kblok
Copy link
Contributor Author

kblok commented May 7, 2021

@Androbin, did #7060 solve this? can I close it?

@Androbin
Copy link
Contributor

Androbin commented May 7, 2021

@kblok #7060 solved the issue underlying the need the disable the cache.
If we implement this fix in PuppeteerSharp, we can simplify write:

_cacheDisabled(): boolean {
  return this._userCacheDisabled;
}

@kblok kblok closed this as completed May 7, 2021
@Androbin
Copy link
Contributor

Androbin commented Jul 4, 2021

See hardkoded/puppeteer-sharp#1765

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

No branches or pull requests

4 participants