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

chore/fix: Upgrade node to 20.11.0 #2398

Merged
merged 1 commit into from
Feb 21, 2024
Merged

Conversation

joshlarson
Copy link
Contributor

This fixes an issue where running individual tests sometimes fails with this error:

node:internal/assert:14 throw new ERR_INTERNAL_ASSERTION(message);

More info here: nodejs/node#48763
And here: nodejs/node#50383

Copy link

Coverage of commit 64b427f

Summary coverage rate:
  lines......: 94.3% (3124 of 3313 lines)
  functions..: 73.5% (1295 of 1763 functions)
  branches...: no data found

Files changed coverage rate: n/a

Download coverage report

@lemald
Copy link
Member

lemald commented Feb 15, 2024

Really hyped if this ends up fixing it, this error has bugged me for a while!

Question: is this fix available anywhere in the 20.x branch or do we have to go to 21? I just ask because the even-numbered releases get an LTS period so 21 will actually become unsupported sooner than 20 per the schedule.

@joshlarson
Copy link
Contributor Author

I will dig some more - I just picked the most recent version, but I think it did get resolved somewhere in 20.x.

@joshlarson joshlarson force-pushed the jdl/fix/upgrade-node-version branch 2 times, most recently from 7470718 to 6ee0eab Compare February 15, 2024 17:22
@joshlarson joshlarson changed the title fix: Upgrade node to 21.6.0 fix: Upgrade node to 20.11.1 Feb 15, 2024
@joshlarson
Copy link
Contributor Author

@lemald I replaced it with the latest node 20.x version and that still worked! Thanks for catching that!

@joshlarson joshlarson marked this pull request as ready for review February 15, 2024 17:24
@joshlarson joshlarson requested a review from a team as a code owner February 15, 2024 17:24
@lemald
Copy link
Member

lemald commented Feb 15, 2024

@joshlarson sorry, the one other place you'll need to update is the Dockerfile, since that references a particular version of the Node image. See #2193 (but you shouldn't need any changes to package.json or the lockfile for this upgrade).

Copy link

Coverage of commit 6ee0eab

Summary coverage rate:
  lines......: 94.3% (3125 of 3313 lines)
  functions..: 73.5% (1296 of 1763 functions)
  branches...: no data found

Files changed coverage rate: n/a

Download coverage report

Copy link

Coverage of commit 6ee0eab

Summary coverage rate:
  lines......: 94.3% (3125 of 3313 lines)
  functions..: 73.5% (1296 of 1763 functions)
  branches...: no data found

Files changed coverage rate: n/a

Download coverage report

@joshlarson joshlarson force-pushed the jdl/fix/upgrade-node-version branch 2 times, most recently from 0eb30eb to 9896916 Compare February 15, 2024 19:50
This fixes an issue where running individual tests sometimes fails
with this error:

node:internal/assert:14 throw new ERR_INTERNAL_ASSERTION(message);

More info here: nodejs/node#48763
And here: nodejs/node#50383
@joshlarson joshlarson changed the title fix: Upgrade node to 20.11.1 chore/fix: Upgrade node to 20.11.0 Feb 15, 2024
@joshlarson
Copy link
Contributor Author

@lemald Updated, I believe!

Copy link

Coverage of commit 081bfda

Summary coverage rate:
  lines......: 94.5% (3131 of 3313 lines)
  functions..: 73.5% (1296 of 1763 functions)
  branches...: no data found

Files changed coverage rate: n/a

Download coverage report

Copy link

Coverage of commit 081bfda

Summary coverage rate:
  lines......: 94.4% (3126 of 3313 lines)
  functions..: 73.5% (1296 of 1763 functions)
  branches...: no data found

Files changed coverage rate: n/a

Download coverage report

Copy link

Coverage of commit 081bfda

Summary coverage rate:
  lines......: 94.3% (3125 of 3313 lines)
  functions..: 73.5% (1295 of 1763 functions)
  branches...: no data found

Files changed coverage rate: n/a

Download coverage report

Copy link
Member

@lemald lemald left a comment

Choose a reason for hiding this comment

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

Confirmed locally that the Docker build works.

@joshlarson
Copy link
Contributor Author

Oh - I was wrong about 20.11.0 fixing it, and I couldn't find a 20.11.1 alpine docker image (I'm also not sure if 20.11.1 even fixes it)

I'll probably merge this as-is, because while it didn't 100% fix it, it did seem to reduce the probability that it was happening 🤯 (maybe that's just confirmation bias)

@lemald
Copy link
Member

lemald commented Feb 16, 2024

Oh - I was wrong about 20.11.0 fixing it, and I couldn't find a 20.11.1 alpine docker image (I'm also not sure if 20.11.1 even fixes it)

I'll probably merge this as-is, because while it didn't 100% fix it, it did seem to reduce the probability that it was happening 🤯 (maybe that's just confirmation bias)

Ah, that's unfortunate but I agree it's still worth merging this, doesn't hurt to keep our dependencies up-to-date.

@joshlarson joshlarson merged commit 63c5999 into main Feb 21, 2024
9 checks passed
@joshlarson joshlarson deleted the jdl/fix/upgrade-node-version branch February 21, 2024 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants