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

fix(webkit): swallow requests from detached frames #6242

Merged
merged 2 commits into from
May 3, 2021

Conversation

JoelEinbinder
Copy link
Contributor

@JoelEinbinder JoelEinbinder commented Apr 20, 2021

I tried to make a test to repro this by sending hundreds of requests from a frame with redirects, and while they were inflight removing the frame, but I couldn't get it to happen. However the user stack trace is pretty strong evidence that sometimes network requests come in for frames that have been detached. If its a redirect, I get the frame from the first request. Otherwise the request is silently swallowed instead of crashing with unhandled promise rejection.

Fixes #6144.

// sometimes we get stray network events for detached frames
// TODO(einbinder) why?
if (!frame)
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's also update line 928 to continue all requests that we did not find in the map, to avoid stalling requests that did not have frameId for some reason.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I canceled them instead of continuing, to avoid accidentally purchasing items via requests that were supposed to be intercepted.

@JoelEinbinder JoelEinbinder merged commit 6219042 into microsoft:master May 3, 2021
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

Successfully merging this pull request may close these issues.

[BUG] UnhandledPromiseRejectionWarning in CI logs
2 participants