webdriverio-community / node-edgedriver

Microsofts' EdgeDriver for Node.js
https://developer.microsoft.com/en-us/microsoft-edge/tools/webdriver/
MIT License
5 stars 9 forks source link

If Edgedriver doesn't exist for installed Edge browser but a different Edgedriver is already installed, don't fail in download #365

Closed danielhjacobs closed 1 month ago

danielhjacobs commented 1 month ago

https://github.com/actions/runner-images/blob/main/images/ubuntu/Ubuntu2204-Readme.md#browsers-and-drivers had Microsoft Edge 126.0.2592.113 installed and Microsoft Edge WebDriver 126.0.2592.102 installed. At the time, https://msedgedriver.azureedge.net/126.0.2592.113/edgedriver_linux64.zip did not exist. The job https://github.com/ruffle-rs/ruffle/actions/runs/10068053329/job/27832705746 said "Detected Microsoft Edge v126.0.2592.113" and then after "Downloading Edgedriver from https://msedgedriver.azureedge.net/126.0.2592.113/edgedriver_linux64.zip" said "Error: End of central directory not found".

That Linux build does now exist, but let's imagine a different scenario. https://packages.microsoft.com/repos/edge/pool/main/m/microsoft-edge-stable/ has 127.0.2651.74 as the newest Edge version. However, https://msedgedriver.azureedge.net/LATEST_RELEASE_127_LINUX is 127.0.2651.72. https://msedgedriver.azureedge.net/127.0.2651.74/edgedriver_linux64.zip is now what https://msedgedriver.azureedge.net/126.0.2592.113/ was then.

That's exactly the Edge and Edgedriver versions in https://github.com/actions/runner-images/pull/10376

If an Edgedriver doesn't exist for the installed Edge browser, instead of failing when the corresponding Edgedriver version does not exist, if a different Edgedriver is already installed, the download function should just return that.

Corresponding bugs include the following:

danielhjacobs commented 1 month ago

This error is not thrown because https://msedgedriver.azureedge.net/127.0.2651.74/edgedriver_linux64.zip (and any other Edgedriver URL that doesn't actually exist) returns an XML body: https://github.com/webdriverio-community/node-edgedriver/blob/7b2b44946f422b86e4c937d277f412a380394a1c/src/install.ts#L49

This is the XML body:

<Error>
<Code>BlobNotFound</Code>
<Message>
The specified blob does not exist. RequestId:f1c70ad1-a01e-0010-3119-e4ad68000000 Time:2024-08-01T13:48:46.8703710Z
</Message>
</Error>

Instead, the error comes from here: https://github.com/webdriverio-community/node-edgedriver/blob/7b2b44946f422b86e4c937d277f412a380394a1c/src/install.ts#L164

The message is Error: End of central directory not found

danielhjacobs commented 1 month ago

Actually, I just realized something:

https://github.com/actions/runner-images/blob/ubuntu22/20240721.1/images/ubuntu/Ubuntu2204-Readme.md#environment-variables-1 sets the EDGEWEBDRIVER environment variable

But this uses process.env.EDGEDRIVER_VERSION

https://github.com/webdriverio-community/node-edgedriver/blob/7b2b44946f422b86e4c937d277f412a380394a1c/src/install.ts#L24

christian-bromann commented 1 month ago

@danielhjacobs thanks for reporting, any contributions are appreciated!

danielhjacobs commented 1 month ago

I opened https://github.com/actions/runner-images/issues/10380, would them addressing that fix the issue?

christian-bromann commented 1 month ago

@danielhjacobs I've implemented a workaround that fetches the latest available Edgedriver for given major version, this is how the logs look like in those case:

2024-08-04T06:55:03.097Z INFO edgedriver: Detected Microsoft Edge v127.0.2651.74
2024-08-04T06:55:03.098Z INFO edgedriver: Downloading Edgedriver from https://msedgedriver.azureedge.net/127.0.2651.74/edgedriver_linux64.zip
2024-08-04T06:55:03.349Z ERROR edgedriver: Failed to download Edgedriver: Failed to download binary from https://msedgedriver.azureedge.net/127.0.2651.74/edgedriver_linux64.zip (statusCode 404), trying alternative download URL...
2024-08-04T06:55:03.349Z INFO edgedriver: Attempt to fetch latest v127 for linux from https://msedgewebdriverstorage.blob.core.windows.net/edgewebdriver?delimiter=%2F&maxresults=2500&restype=container&comp=list&_=1722752[48](https://github.com/webdriverio-community/node-edgedriver/actions/runs/10234266086/job/28313517695#step:6:49)3611&timeout=60000
2024-08-04T06:55:03.885Z INFO edgedriver: Downloading alternative Edgedriver version from https://msedgewebdriverstorage.blob.core.windows.net/edgewebdriver/LATEST_RELEASE_127_LINUX
2024-08-04T06:55:03.940Z INFO edgedriver: Downloading Edgedriver from https://msedgedriver.azureedge.net/127.0.2651.87/edgedriver_linux64.zip
2024-08-04T06:55:04.392Z INFO edgedriver: Finished downloading Edgedriver
2024-08-04T06:55:04.[49](https://github.com/webdriverio-community/node-edgedriver/actions/runs/10234266086/job/28313517695#step:6:50)4Z INFO edgedriver: Starting EdgeDriver at /tmp/msedgedriver with params: --port=4444 --allowed-origins=* --allowed-ips=
christian-bromann commented 1 month ago

A fix was released in v5.6.1

danielhjacobs commented 1 month ago

I see that bump was in https://github.com/webdriverio/webdriverio/commit/a51e739fdb25f62bef70e0f54c6998d15adb0bb1 but I don't see that commit in https://github.com/webdriverio/webdriverio/compare/v8.39.1...v8.40.0, unless I'm missing it. Will that be included in a new version of webdriverio anytime soon?

christian-bromann commented 1 month ago

Will that be included in a new version of webdriverio anytime soon?

There is no need to make any changes in the core repository as Edgedriver is defined with a ^ so your project should pick it up once you update your dependencies.

danielhjacobs commented 1 month ago

Ah, I think we used deprecated methods for testing on the browsers:

https://github.com/ruffle-rs/ruffle/blob/2afdeb77a38e8af5803fee1bc90e056980ea0583/web/package.json#L40-L42

We should do npm remove wdio-chromedriver-service wdio-edgedriver-service wdio-geckodriver-service, npm install --save-dev chromedriver edgedriver geckodriver and npm install webdriverio, right?

christian-bromann commented 1 month ago

Yeah, with webdriverio@v8.14.0 and higher we eliminated the need to have services for starting browsers, read more on this in our blog post.