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

build: improve update script error handling #832

Merged
merged 3 commits into from
Mar 15, 2023

Conversation

MikeMcC399
Copy link
Collaborator

This PR improves the error handling for the update scripts called by

npm run update:cypress

It was previously possible for them to fail silently, particularly if pnpm or yarn were not installed. This situation can occur if nvm is used to manage Node.js versions, since global installations of pnpm and yarn are tied to a specific Node.js version and not carried over, for instance if Node.js is updated to the latest LTS version.

The scripts now check that the following commands are available:

  • npm
  • pnpm
  • yarn

and advise installation if they are missing.

Verification

On Ubuntu and Windows 11 Pro.

  1. With no npm available (for instance by uninstalling nvm). npm -v should produce "command not found", execute
./scripts/update-cypress-latest.sh

and confirm output of message to install Node.js or nvm.

  1. Install Node.js LTS (currently 18.5.0) either directly or via nvm.

On Windows execute

npm config set script-shell "C:\\Program Files\\git\\bin\\bash.exe" --location user

Execute

npm ci
npm run update:cypress

Confirm that the script informs that npm is installed, and that pnpm and yarn are not installed.

  1. Follow the instructions to install pnpm and execute the script again.

Confirm that the script informs that npm is installed, and that yarn is not installed.

  1. Follow the instructions to install yarn and execute the script again.

This time the script should run to completion and update all examples to use cypress@12.8.0 (excepting those in the examples/v9 directory).

@MikeMcC399 MikeMcC399 marked this pull request as ready for review March 15, 2023 10:22
Copy link
Contributor

@AtofStryker AtofStryker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I wonder if the script could also check if nvm is installed and if not, install it and check if the system node is set to lts, otherwise bail

scripts/check-package-manager-npm.sh Show resolved Hide resolved
@AtofStryker
Copy link
Contributor

@lmiller1990 you mind taking a look at this PR? the Node v19 job for Cypress 9 is failing since the actual .gz file 404s on Node's site. I'm not sure why this isn't failing for the Cypress@12 jobs though for v19.

@MikeMcC399
Copy link
Collaborator Author

MikeMcC399 commented Mar 15, 2023

@AtofStryker

the Node v19 job for Cypress 9 is failing since the actual .gz file 404s on Node's site. I'm not sure why this isn't failing for the Cypress@12 jobs though for v19.

The results of .github/workflows/example-node-versions.yml are flaky for multiple PRs.

In the run for this PR https://github.com/cypress-io/github-action/actions/runs/4429718318

https://github.com/cypress-io/github-action/actions/runs/4429718318/jobs/7770543697 fails with

Attempting to download 19...
Not found in manifest. Falling back to download directly from Node
Acquiring 19.[8](https://github.com/cypress-io/github-action/actions/runs/4429718318/jobs/7770543697#step:2:9).1 - x64 from https://nodejs.org/dist/v1[9](https://github.com/cypress-io/github-action/actions/runs/4429718318/jobs/7770543697#step:2:10).8.1/node-v19.8.1-linux-x64.tar.gz
Downloading only node binary from https://nodejs.org/dist/v19.8.1/win-x64/node.exe
Error: Unexpected HTTP response: 404

which doesn't quite make sense because this is an Ubuntu runner, not a win-x64 one


https://github.com/cypress-io/github-action/actions/runs/4429718318/jobs/7770541047 succeeds, although there shouldn't be any difference between this run and the previous one which failed;

Attempting to download 19...
Not found in manifest. Falling back to download directly from Node
Acquiring 19.[8](https://github.com/cypress-io/github-action/actions/runs/4429718318/jobs/7770541047#step:2:9).1 - x64 from https://nodejs.org/dist/v1[9](https://github.com/cypress-io/github-action/actions/runs/4429718318/jobs/7770541047#step:2:10).8.1/node-v19.8.1-linux-x64.tar.gz
Extracting ...
/usr/bin/tar xz --strip 1 --warning=no-unknown-keyword -C /home/runner/work/_temp/fc199[11](https://github.com/cypress-io/github-action/actions/runs/4429718318/jobs/7770541047#step:2:12)2-ab1c-4734-a359-3f80dd99f0bc -f /home/runner/work/_temp/bcad[14](https://github.com/cypress-io/github-action/actions/runs/4429718318/jobs/7770541047#step:2:15)a7-7d05-4486-bc62-cb283a[17](https://github.com/cypress-io/github-action/actions/runs/4429718318/jobs/7770541047#step:2:18)27de
Adding to the cache ...
Done
Environment details
  node: v19.8.1
  npm: 9.5.1
  yarn: 1.22.19

I'm not sure if this is a GitHub issue or a Node.js issue at the moment. It could be a flaky node or load-balancer for instance.

If it's still happening tomorrow I would take the problem to GitHub / Node.js support to look at.


$ curl -I https://nodejs.org/dist/v19.8.1/node-v19.8.1-linux-s390x.tar.xz
HTTP/2 200
date: Wed, 15 Mar 2023 19:14:24 GMT
content-type: application/x-xz
content-length: 23005624
last-modified: Wed, 15 Mar 2023 16:14:44 GMT
etag: "6411eef4-15f09b8"
cache-control: max-age=14400
cf-cache-status: HIT
age: 327
accept-ranges: bytes
strict-transport-security: max-age=31536000; includeSubDomains; preload
x-content-type-options: nosniff
server: cloudflare
cf-ray: 7a871446ede7913d-FRA

$ curl -I https://nodejs.org/dist/v19.8.1/win-x64/node.exe
HTTP/2 404
date: Wed, 15 Mar 2023 19:15:04 GMT
content-type: text/html
cache-control: max-age=14400
cf-cache-status: HIT
age: 4484
strict-transport-security: max-age=31536000; includeSubDomains; preload
x-content-type-options: nosniff
server: cloudflare
cf-ray: 7a8715434cc45c98-FRA

@AtofStryker
Copy link
Contributor

I'm not sure if this is a GitHub issue or a Node.js issue at the moment. It could be a flaky node or load-balancer for instance.

If it's still happening tomorrow I would take the problem to GitHub / Node.js support to look at.

I think its a Node.js issue. When I actually try to download the file itself from the browser it fails with a 404. Almost seems like a faulty load balancer or something of sorts. If it is passing for the Cypress latest v19 job, I would assume the load balancer. My guess is this should be fixed either by a node version bump or someone adjusting the load balancer in the near future.

I wouldn't worry too much about it, but as you said if it is still happening tomorrow it might be worth looking at, but I don't know how we would be able to fix that besides filing an issue.

@lmiller1990 lmiller1990 merged commit 8d34399 into cypress-io:master Mar 15, 2023
@MikeMcC399 MikeMcC399 deleted the build/update-script branch March 16, 2023 08:39
@MikeMcC399
Copy link
Collaborator Author

@AtofStryker
curl -I https://nodejs.org/dist/v19.8.1/win-x64/node.exe today produces an HTTP/2 200 response, whereas yesterday it was responding with HTTP/2 404, so it seems that Node.js / cloudflare have fixed their issue.

@github-actions
Copy link

🎉 This PR is included in version 5.4.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@AtofStryker AtofStryker removed their assignment Mar 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants