Change error handling #185

Open
soulgalore opened this Issue Sep 5, 2016 · 4 comments

Projects

None yet

1 participant

@soulgalore
Member

I got two things I think could be a good change on how we handle errors. There is specific two errors that I want to handle in a different way:

  • With Firefox the HAR Export Trigger fails sometimes, it's in Firefox/the trigger and nothing we can do to fix without fixing the original problem.

    [2016-09-05 18:35:30] Couldn't generate the HAR from Firefox from the HAR Export Trigger.
    [2016-09-05 18:35:30] ScriptTimeoutError: Timed out
    ...
    From: Task: WebDriver.executeScript()
    at Driver.schedule (/usr/src/app/node_modules/selenium-webdriver/lib/webdriver.js:414:17)
    at Driver.executeAsyncScript (/usr/src/app/node_modules/selenium-webdriver/lib/webdriver.js:651:17)
    at Promise.try (/usr/src/app/node_modules/browsertime/lib/core/seleniumRunner.js:186:28)
    

    I think we should not throw the error, just log and make sure we document that the HAR can be missing if it fails. It will help tools like sitespeed.io that still will get a lot of info running the scripts in Firefox and the HAR is just some extra info.

  • If the page load time times out, we could still execute the JS and return what we have:

    [2016-09-04 23:35:48] WebDriverError:TimeoutError: Error loading page, timed out (checkLoad)
    [2016-09-04 23:35:48] Could not load URLUrlLoadError: Failed to load http://www.cnet.com/, cause: Error loading page,
    timed out (checkLoad)
    ...
    at getUrl.then.then.catch.catch (/usr/src/app/node_modules/browsertime/lib/core/seleniumRunner.js:147:15)
      at tryCatcher (/usr/src/app/node_modules/browsertime/node_modules/bluebird/js/release/util.js:16:23)
    

    Browsertime could still signal an error but with the data that we could fetch.

@soulgalore soulgalore added the 1.0 label Sep 5, 2016
@soulgalore soulgalore changed the title from Changed error handling to Change error handling Sep 5, 2016
@soulgalore soulgalore referenced this issue in sitespeedio/sitespeed.io Sep 5, 2016
Closed

Scripted timeout - for some pages #1146

@soulgalore
Member

For the second error, I think we should jus change the default Javascript that checks if a page is ready or not. Today we only check loadEventEnd but we could make the check end after x minutes.

@soulgalore
Member

No that will not work since Selenium is waiting for on load to happen before running our Javascript.

@soulgalore soulgalore added the upstream label Sep 6, 2016
@soulgalore
Member

We need to be able to change pageLoadStrategies in Selenium to be able to create our own wait for the driver load:

Firefox Marionette: https://bugzilla.mozilla.org/show_bug.cgi?id=937659
Chrome Chromedriver: https://groups.google.com/a/chromium.org/forum/m/#!topic/devtools-reviews/Hu74sHnkWcM

and then add it to Selenium.

@soulgalore soulgalore removed the upstream label Sep 7, 2016
@soulgalore
Member

Lets move the change in pageLoadStrategies to #186 and this issue will only follow the HAR for Firefox.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment