search

xvrh / puppeteer-dart

A Dart library to automate the Chrome browser over the DevTools Protocol. This is a port of the Puppeteer API
BSD 3-Clause "New" or "Revised" License
235 stars 59 forks source link

Fix navigation error #312

Closed kendalsickels-wf closed 6 months ago

kendalsickels-wf commented 6 months ago

@xvrh We've started to see an issue similar to https://github.com/xvrh/puppeteer-dart/issues/237 in our tests. I was able to create a test that fails consistently without any changes committed.

Then committed some changes that work around the issue but I'm lacking the context that you likely have over the entirety of this project to say if those changes would cause issues elsewhere.

I operated under the assumption that in the _onExecutionContextsCleared scenario we should go quietly into the night instead of erroring and _onExecutionContextsDestroyed we should error. But perhaps that was a poor assumption. Looking for any guidance/insight you might have on how to solve this issue.

codecov[bot] commented 6 months ago

Codecov Report

Attention: Patch coverage is 66.66667% with 5 lines in your changes are missing coverage. Please review.

:exclamation: No coverage uploaded for pull request base (master@3fe1eeb). Click here to learn what that means.

Files Patch % Lines
lib/src/page/dom_world.dart 69.23% 4 Missing :warning:
lib/src/page/frame_manager.dart 50.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #312 +/- ## ========================================= Coverage ? 88.80% ========================================= Files ? 32 Lines ? 3172 Branches ? 0 ========================================= Hits ? 2817 Misses ? 355 Partials ? 0 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

xvrh commented 6 months ago

Thanks for the contribution. I'll merge it as it: all the tests are passing and it seems to fix already some other issues (like https://github.com/xvrh/puppeteer-dart/issues/314)