NekR / offline-plugin

Offline plugin (ServiceWorker, AppCache) for webpack (https://webpack.js.org/)
MIT License
4.52k stars 295 forks source link

Fix cache key #398

Closed yesmeck closed 6 years ago

yesmeck commented 6 years ago

Recently we received some reports that SW caches the wrong file (https://github.com/ant-design/ant-design/issues/11080). After some investigations, I found that if a URL request failed, then the failed request will be cached by another response. The reason is that when a request failed, the responses will be filtered in https://github.com/NekR/offline-plugin/blob/4553b5f5a6a02626b1985db2eb03f1f3622b859a/src/misc/sw-template.js#L622, but the requests are not filtered, so that responses get a different length than the reuqets which resulting in this line https://github.com/NekR/offline-plugin/blob/4553b5f5a6a02626b1985db2eb03f1f3622b859a/src/misc/sw-template.js#L627 get a wrong request for respone.

yesmeck commented 6 years ago

Have no idea how to fix tests.

GGAlanSmithee commented 6 years ago

@NekR this does seems lik a bug too me, but I don´t know enough about the intended logic too say for sure.

Maybe the correct way to handle this would be to perform a corresponding filtering on requests too make sure they are synced with responses?

if (!failAll) {
    responses = responses.filter(data => !data.error);
    requests = requests.filter(/* where corresponding respons exists */)
}
yesmeck commented 6 years ago

@GGAlanSmithee filtering on request could be a fix too.

yesmeck commented 6 years ago

Use response.url as the key is more straightforward.

GGAlanSmithee commented 6 years ago

Use response.url as the key is more straightforward.

This seems reasonable.

NekR commented 6 years ago

This indeed seems to be a bug. Unfortunately response.url is unreliable option, since if there is a redirect, response.url will be different.

I'll do the fix. Thanks for reporting this 👍