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
fix(network): don't disable cache for auth challenge #6962
Conversation
@OrKoN could you PTAL? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change LGTM. It seems there are no current tests for this logic. Would it be possible to add the tests so that we capture the right expectations and no one changes the logic back if it does not work for some use cases?
@OrKoN I added a unit test covering this logic. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
#729 introduced
_updateProtocolRequestInterception
Objective: Call
Network.setRequestInterceptionEnabled
for auth challenge or if requested by user#1154 introduced
Network.setCacheDisabled
Objective: Ensure custom user response handling is always called
#4260 introduced
_updateProtocolCacheDisabled
Objective: Call
Network.setCacheDisabled
if requested by userThe cache and request interception work on the network level, while we want the custom user response handling to work independent of the network, which is why we're currently forced to disable the cache in some cases.
As long as there is no custom user response handling, it should be unnecessary to disable the cache for the auth challenge, since it only needs to be called in case of an actual network request anyway.
Related issue: #2905