Temporary fix for deadlock on pagehide event evaluation when page.close is called #979
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This new EvaluateWithContext will allow us to pass in a context with a timeout. For issue #971, we're seeing that often the evaluate to hide the page on page.close ends up in a deadlock (waiting for the response from the chrome browser after executing the CDP evaluate command). We are not sure of the root cause of the issue, but this fix will help prevent this deadlock allow the test run to complete in a timely manner without it causing a time out error.
Now we can use the EvaluateWithContext from page.close when we call evaluate to hide the page. We're going to rely on the default timeout for now, and refine the timeout if we see this occurring too often -- if we see this occur too often this will affect the test result depending on the default timeout (which is 30 seconds unless overridden by the user).
inancgumus
approved these changes
Jul 26, 2023
There was a problem hiding this comment.
That's interesting! Have you tried using ExecuteWithoutExpectationOnReply?
That is a good solution, and a cleaner way in avoiding having to wait on the response. I did consider a similar approach, but my concern with that was that we would hide the problem, but this is only meant as a temporary fix with a constant reminder (whenever we see the warning) that there is an underlying issue that needs to be dealt with. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of changes
When we run the k6 browser module against a remote chrome instance, we've found that 1 or more VUs deadlock when running the test in this issue. What we have come to realise during the investigation is that the evaluate on the
pagehideevent whenpage.closeis called seems to wait indefinitely for a response from chrome, that never seems to arrive.This is a temporary fix where we use the page's
defaultTimeoutto timeout thisevaluaterequest instead of waiting indefinitely. We still need to investigate the root cause of the issue. The downside of this fix is that when thisevaluatedoes timeout then we may not receive all the web vital measurements for that iteration (the reason we added thepagehideevent evaluate was to force the web vital measurements to be calculated and sent to the browser module: #953).Doesn't close issue #971 but temporarily alleviates the issue.
Checklist