Repository navigation
Conversation
|
You can preview db46964 at https://pr19764-db46964.ngbuilds.io/. |
|
You can preview 18fc7a1 at https://pr19764-18fc7a1.ngbuilds.io/. |
There was a problem hiding this comment.
you no longer clone here, so the comment is above is out of date since cloning happens within the call.
There was a problem hiding this comment.
this change has no tests. how do we know that it works and that we won't break it in the future?
There was a problem hiding this comment.
This code is covered by the existing tests.
The test mocks previously different from the real browser behavior by 1) not exhausting the body on cache.put() and 2) not throwing an error when an exhausted Response body was reused. As part of this same commit, the mocks were updated to have the correct behavior in both cases. This caused the existing tests to fail without the .clone() operation here.
There was a problem hiding this comment.
Can you please add this info into the commit message and document this kind of stuff in the future? Otherwise these micro changes are hard to review and understand in the future. thanks!
|
You can preview a998338 at https://pr19764-a998338.ngbuilds.io/. |
IgorMinar
added
area: service-worker
When Cache.put() is called with a Response, it consumes the response. If the Response is used for any other purpose (such as satisfying the original FetchEvent) it must be cloned first. A bug exists in the mocks used for SW tests, where this condition is not validated. The bodies of MockResponses can be utilized repeatedly without erroring in the same way that a real browser would. This bug is fixed by this commit, which causes tests for the freshness strategy of data caching to start failing. The cause of this failure is a second bug in the data caching code, where the Response is not cloned prior to being passed to Cache.put(). This is also fixed.
PushEvent.data is not the data object itself, but an instance representing the data in wire format, with methods to synchronously decode it to JSON, ArrayBuffer, etc. NGSW assumes all push data is in JSON format.
alxhub
added
action: merge
|
You can preview dcf50d3 at https://pr19764-dcf50d3.ngbuilds.io/. |
PushEvent.data is not the data object itself, but an instance representing the data in wire format, with methods to synchronously decode it to JSON, ArrayBuffer, etc. NGSW assumes all push data is in JSON format. PR Close #19764
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
No description provided.