core(gather-runner): convert PAGE_HUNG to non-fatal runtimeError #9121
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
brendankenny
reviewed
Jun 5, 2019
| @@ -552,6 +552,9 @@ class GatherRunner { | |||
| const passResults = await GatherRunner.runPass(passContext); | |||
| Object.assign(artifacts, passResults.artifacts); | |||
|
|
|||
| // If we encountered a pageLoadError, don't try to keep loading the page in future passes. | |||
| if (passResults.pageLoadError) break; | |||
There was a problem hiding this comment.
this gets a bit weird because we don't populate but still return them. All the other artifacts are either undefined, an Error, or the artifact, but we haven't until now done half-finished artifacts.
Seems ok for now, though
This was referenced Jun 13, 2019
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.
Summary
This should be last step in making all our page-load focused runtimeErrors non-fatal and exiting with an error code. This PR also cleans up some loose ends from #8865 where INSECURE_DOCUMENT_REQUEST was lying around.
Notable Changes
pageLoadErrorand is no longer fatal.{runtimeError: {code: '...'}}pageLoadError, the rest of the passes are skipped and the rest of baseArtifacts are not populated. Necessary for recovery from PAGE_HUNG and just made sense, see core(driver): security errors are no longer a fatal or pageload error #8865 (comment) step 3.clearDataForOriginwill not throw PROTOCOL_TIMEOUT if it can't complete, it will just warn and move on. This is necessary for trying to recover from PAGE_HUNG in a non-fatal way, and it was one of the flagged non-responsive methods in our PROTOCOL_TIMEOUT umbrella anyhow.Related Issues/PRs
followup according to #8865 (comment) (step 2 of the process)