Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
danielhjacobs opened this issue Aug 1, 2024 · 10 comments

Comments

@danielhjacobs
Copy link

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 actions/runner-images#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 danielhjacobs changed the title If Edgedriver doesn't exist for installed Edge browser but a different Edgedriver is already installed, don't fail in parameterless download If Edgedriver doesn't exist for installed Edge browser but a different Edgedriver is already installed, don't fail in download Aug 1, 2024
@danielhjacobs
Copy link
Author

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:

throw new Error(`Failed to download binary (statusCode ${res.status})`)

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:

for (const entry of await zip.getEntries()) {

The message is Error: End of central directory not found

@danielhjacobs
Copy link
Author

danielhjacobs commented Aug 1, 2024

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

edgeVersion: string = process.env.EDGEDRIVER_VERSION,

@christian-bromann
Copy link
Contributor

@danielhjacobs thanks for reporting, any contributions are appreciated!

@danielhjacobs
Copy link
Author

danielhjacobs commented Aug 1, 2024

I opened actions/runner-images#10380, would them addressing that fix the issue?

@christian-bromann
Copy link
Contributor

@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
Copy link
Contributor

A fix was released in v5.6.1

@danielhjacobs
Copy link
Author

I see that bump was in webdriverio/webdriverio@a51e739 but I don't see that commit in webdriverio/webdriverio@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
Copy link
Contributor

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
Copy link
Author

danielhjacobs commented Aug 5, 2024

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
Copy link
Contributor

Yeah, with [email protected] and higher we eliminated the need to have services for starting browsers, read more on this in our blog post.

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

No branches or pull requests

2 participants