"perf(core): change sha1 to xxhash" commit broke runtime of Nest.js application on Ubuntu 20.04 #11071

fiki574 · 2023-02-07T17:51:12Z

Did you read the migration guide?

I have read the whole migration guide

Is there an existing issue that is already proposing this?

I have searched the existing issues

Potential Commit/PR that introduced the regression

7f43821

NestJS version

9.3.2 -> 9.3.5

Describe the regression

So, I've upgraded @nestjs/common and @nestjs/core dependencies from 9.3.2. to 9.3.5. Decided to run them on my PC with Windows 10, runs perfectly, all good. Pushed to my Github and let the Actions complete the build and deployment processes, and this is "docker logs" output of the container that runs my Nest.js application.

Error: Cannot find module '@node-rs/xxhash-linux-x64-gnu'
Require stack:
- /app/node_modules/@node-rs/xxhash/index.js
- /app/node_modules/@nestjs/core/inspector/deterministic-uuid-registry.js
- /app/node_modules/@nestjs/core/inspector/uuid-factory.js
- /app/node_modules/@nestjs/core/injector/instance-wrapper.js
- /app/node_modules/@nestjs/core/injector/injector.js
- /app/node_modules/@nestjs/core/injector/module-ref.js
- /app/node_modules/@nestjs/core/injector/lazy-module-loader/lazy-module-loader.js
- /app/node_modules/@nestjs/core/inspector/serialized-graph.js
- /app/node_modules/@nestjs/core/injector/container.js
- /app/node_modules/@nestjs/core/injector/index.js
- /app/node_modules/@nestjs/core/index.js
- /app/dist/main.js
    at Module._resolveFilename (node:internal/modules/cjs/loader:1047:15)
    at Module._load (node:internal/modules/cjs/loader:893:27)
    at Module.require (node:internal/modules/cjs/loader:1113:19)
    at require (node:internal/modules/cjs/helpers:103:18)
    at Object.<anonymous> (/app/node_modules/@node-rs/xxhash/index.js:172:31)
    at Module._compile (node:internal/modules/cjs/loader:1226:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1280:10)
    at Module.load (node:internal/modules/cjs/loader:1089:32)
    at Module._load (node:internal/modules/cjs/loader:930:12)
    at Module.require (node:internal/modules/cjs/loader:1113:19) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/app/node_modules/@node-rs/xxhash/index.js',
    '/app/node_modules/@nestjs/core/inspector/deterministic-uuid-registry.js',
    '/app/node_modules/@nestjs/core/inspector/uuid-factory.js',
    '/app/node_modules/@nestjs/core/injector/instance-wrapper.js',
    '/app/node_modules/@nestjs/core/injector/injector.js',
    '/app/node_modules/@nestjs/core/injector/module-ref.js',
    '/app/node_modules/@nestjs/core/injector/lazy-module-loader/lazy-module-loader.js',
    '/app/node_modules/@nestjs/core/inspector/serialized-graph.js',
    '/app/node_modules/@nestjs/core/injector/container.js',
    '/app/node_modules/@nestjs/core/injector/index.js',
    '/app/node_modules/@nestjs/core/index.js',
    '/app/dist/main.js'
  ]
}

Minimum reproduction code

No response

Input code

Expected behavior

Application should run normally on Ubuntu 20.04, just like it does on Windows 10.

Other

Runs on Github hosted Ubuntu runners, this is Actions response for a simple unit test stage:

Produces pretty much the same error:

Require stack:
      node_modules/@node-rs/xxhash/index.js
      node_modules/@nestjs/core/inspector/deterministic-uuid-registry.js
      node_modules/@nestjs/core/inspector/uuid-factory.js
      node_modules/@nestjs/core/injector/instance-wrapper.js
      node_modules/@nestjs/core/injector/injector.js
      node_modules/@nestjs/core/injector/module-ref.js
      node_modules/@nestjs/core/injector/lazy-module-loader/lazy-module-loader.js
      node_modules/@nestjs/core/inspector/serialized-graph.js
      node_modules/@nestjs/core/injector/container.js
      node_modules/@nestjs/testing/testing-module.builder.js
      node_modules/@nestjs/testing/test.js
      node_modules/@nestjs/testing/index.js
      src/tests/auth.controller.spec.ts

    > 1 | import { Test, TestingModule } from "@nestjs/testing";
        | ^
      2 | import { I18nContext } from "nestjs-i18n";
      3 | import { getMockReq, getMockRes } from "@jest-mock/express";
      4 | import { UserService } from "@user/services";

      at Resolver._throwModNotFoundError (node_modules/jest-resolve/build/resolver.js:427:11)
      at Object.<anonymous> (node_modules/@node-rs/xxhash/index.js:[17](https://github.com/Pinehood/ph-s2u-backend/actions/runs/4116536300/jobs/7106703742#step:7:18)2:31)
      at Object.<anonymous> (node_modules/@nestjs/core/inspector/deterministic-uuid-registry.js:4:[18](https://github.com/Pinehood/ph-s2u-backend/actions/runs/4116536300/jobs/7106703742#step:7:19))
      at Object.<anonymous> (node_modules/@nestjs/core/inspector/uuid-factory.js:5:39)
      at Object.<anonymous> (node_modules/@nestjs/core/injector/instance-wrapper.js:11:24)
      at Object.<anonymous> (node_modules/@nestjs/core/injector/injector.js:14:28)
      at Object.<anonymous> (node_modules/@nestjs/core/injector/module-ref.js:8:[20](https://github.com/Pinehood/ph-s2u-backend/actions/runs/4116536300/jobs/7106703742#step:7:21))
      at Object.<anonymous> (node_modules/@nestjs/core/injector/lazy-module-loader/lazy-module-loader.js:5:22)
      at Object.<anonymous> (node_modules/@nestjs/core/inspector/serialized-graph.js:8:30)
      at Object.<anonymous> (node_modules/@nestjs/core/injector/container.js:6:28)
      at Object.<anonymous> (node_modules/@nestjs/testing/testing-module.builder.js:6:[21](https://github.com/Pinehood/ph-s2u-backend/actions/runs/4116536300/jobs/7106703742#step:7:22))
      at Object.<anonymous> (node_modules/@nestjs/testing/test.js:5:34)
      at Object.<anonymous> (node_modules/@nestjs/testing/index.js:11:[22](https://github.com/Pinehood/ph-s2u-backend/actions/runs/4116536300/jobs/7106703742#step:7:23))
      at Object.<anonymous> (src/tests/auth.controller.spec.ts:1:1)

The text was updated successfully, but these errors were encountered:

micalevisk · 2023-02-07T17:56:10Z

it works on my Ubuntu 20.04 and on my CI pipeline on GH which uses Ubuntu.

Could you share your workflow file?

fiki574 · 2023-02-07T18:00:06Z

it works on my Ubuntu 20.04 and on my CI pipeline on GH which uses Ubuntu.

Could you share your workflow file?

Here is my package.json part with dependencies and all versions:

"dependencies": {
    "@googlemaps/google-maps-services-js": "3.3.26",
    "@nestjs/common": "9.3.5",
    "@nestjs/config": "2.3.0",
    "@nestjs/core": "9.3.5",
    "@nestjs/event-emitter": "1.4.1",
    "@nestjs/jwt": "10.0.1",
    "@nestjs/passport": "9.0.1",
    "@nestjs/platform-express": "9.3.5",
    "@nestjs/schedule": "2.2.0",
    "@nestjs/swagger": "6.2.1",
    "@nestjs/terminus": "9.2.0",
    "@nestjs/throttler": "4.0.0",
    "@nestjs/typeorm": "9.0.1",
    "@sendgrid/mail": "7.7.0",
    "@willsoto/nestjs-prometheus": "5.1.0",
    "aws-sdk": "2.1310.0",
    "class-transformer": "0.5.1",
    "class-validator": "0.13.2",
    "cookie-parser": "1.4.6",
    "express-basic-auth": "1.2.1",
    "googleapis": "110.0.0",
    "handlebars": "4.7.7",
    "ioredis": "5.3.0",
    "nestjs-http-promise": "2.0.0",
    "nestjs-i18n": "10.2.6",
    "nestjs-pino": "3.1.2",
    "passport-jwt": "4.0.1",
    "pg": "8.9.0",
    "pino-pretty": "9.1.1",
    "prom-client": "14.1.1",
    "reflect-metadata": "0.1.13",
    "rimraf": "4.1.2",
    "rxjs": "7.8.0",
    "swagger-stats": "0.99.5",
    "swagger-ui-express": "4.6.0",
    "twilio": "3.84.1",
    "typeorm": "0.3.12",
    "typeorm-naming-strategies": "4.1.0"
  },
  "devDependencies": {
    "@jest-mock/express": "2.0.1",
    "@nestjs/cli": "9.2.0",
    "@nestjs/schematics": "9.0.4",
    "@nestjs/testing": "9.3.5",
    "@types/cookie-parser": "1.4.3",
    "@types/cron": "2.0.0",
    "@types/express": "4.17.17",
    "@types/express-status-monitor": "1.3.0",
    "@types/jest": "29.4.0",
    "@types/multer": "1.4.7",
    "@types/node": "18.13.0",
    "@types/passport-jwt": "3.0.8",
    "@types/supertest": "2.0.12",
    "@types/swagger-stats": "0.95.8",
    "@typescript-eslint/eslint-plugin": "5.51.0",
    "@typescript-eslint/parser": "5.51.0",
    "eslint": "8.33.0",
    "eslint-config-prettier": "8.6.0",
    "eslint-plugin-import": "2.27.5",
    "jest": "29.4.2",
    "prettier": "2.8.3",
    "supertest": "6.3.3",
    "ts-jest": "29.0.5",
    "ts-loader": "9.4.2",
    "ts-node": "10.9.1",
    "tsconfig-paths": "4.1.2",
    "tsconfig-paths-jest": "0.0.1",
    "typescript": "4.9.5"
  }

My workflow file:

name: Continuous Integration
on:
  push:
    branches: [develop]
  pull_request:
    branches: [develop]
jobs:
  integrate:
    name: Install, Build, Lint & Test
    runs-on: ubuntu-latest
    steps:
      - name: Checkout
        uses: actions/checkout@v3
      - name: Setup
        uses: actions/setup-node@v3
        with:
          node-version: 18.x
          cache: "npm"
      - name: Install
        run: npm ci
      - name: Build
        run: npm run build
      - name: Lint
        run: npm run lint
      - name: Test
        run: npm run test

jmcdo29 · 2023-02-07T18:04:26Z

I would hazard a guess that @node-rs/xxhash generates different code on Windows 10 and Linux-x64 and the npm ci is just getting the Windows 10 variant due to respecting the lockfile while the runtime is trying to access the linux-x64 specific files. If you could generate a lockfile for ubuntu/linux-x64 that would fix this up.

I'll look into the package and see what they suggest as well. I know we just switched to it for performance purposes

fiki574 · 2023-02-07T18:24:04Z

Right, but this is very problematic for Windows developers, and is a very breaking change to introduce for a simple "small" patch version upgrade, to be fair.

Here's my Dockerfile that might help in reproducing the error:

FROM node:lts-bullseye-slim

WORKDIR /app
COPY . /app

RUN npm ci
RUN npm run build

EXPOSE 3000
CMD ["node", "dist/main"]

For now, I downgraded the problematic dependencies to 9.3.2 from 9.3.5 so all services and pipes I have work now, but would be nice to have an official fix and support of future version normally, without these additional generations and manual steps I need to take for the interop.

Nevertheless, thanks for your work and response time, it's amazing!

H4ad · 2023-02-07T18:29:57Z

From what I have look, napi-rs/xxhash uses the same strategy of esbuild to distribute the packages, xxhash and esbuild.

From what I understand, npm resolves to the correct package depending on the machine that is running the install, if it is Linux 64bits, then linux-x64 will be chosen.

Things can get worse when you mix the architecture of the CI systems, like building in Linux 64 and then deploying in another architecture.

But as fiki said, I think this could be marked as a breaking change because of the way it breaks the deployment process. So I don't know if worth it to recommend fixing the CI or just reverting the package for now and using crypto.createHash('sha1') instead, and then in nestjs 10 we introduce this package.

kamilmysliwiec · 2023-02-07T18:41:17Z

Would you like to create a PR for this @H4ad? If you still have benchmarks comparing sha1 vs xxhash & could link them in the PR that would be ideal (I think you've already shared them with me earlier).

H4ad · 2023-02-07T18:50:28Z

Yes, I can!

About the benchmarks, I will put in the PR but just to give you a idea:

uuid v4 x 14,449,798 ops/sec ±2.08% (75 runs sampled)
uuid v5 x 182,749 ops/sec ±1.26% (80 runs sampled)
sha1 x 396,744 ops/sec ±3.76% (72 runs sampled)
napi-rs/blake x 395,341 ops/sec ±4.28% (65 runs sampled)
nanoid x 2,735,417 ops/sec ±1.01% (78 runs sampled)
uid x 39,619,464 ops/sec ±2.39% (76 runs sampled)
luuked x 3,931,161 ops/sec ±3.93% (73 runs sampled)
napi-rs/uuid x 4,509,946 ops/sec ±1.83% (76 runs sampled)
node-rs/xxHash xxh32 x 3,104,990 ops/sec ±1.90% (76 runs sampled)
node-rs/xxHash xxh64 x 2,985,482 ops/sec ±1.74% (79 runs sampled)
Fastest is uid

sha1 is slower than xxHash but faster than using uuid-v5.

kamilmysliwiec · 2023-02-08T07:55:23Z

Let's track this here #11073

fix(core): removed xxhash to fix #11071

fiki574 added needs triage This issue has not been looked into type: bug 😭 labels Feb 7, 2023

micalevisk mentioned this issue Feb 7, 2023

Test.createTestingModule leaves open handle due to xxh32 #11072

Closed

2 tasks

H4ad added a commit to h4ad-forks/nest that referenced this issue Feb 7, 2023

fix(core): removed xxhash to fix nestjs#11071

3f5dadd

H4ad mentioned this issue Feb 7, 2023

fix(core): removed xxhash to fix #11071 #11073

Merged

12 tasks

H4ad added a commit to h4ad-forks/nest that referenced this issue Feb 7, 2023

fix(core): removed xxhash to fix nestjs#11071

bd92969

urugator mentioned this issue Feb 7, 2023

xxh32 leaves open handle napi-rs/node-rs#658

Open

micalevisk removed the needs triage This issue has not been looked into label Feb 8, 2023

kamilmysliwiec closed this as completed Feb 8, 2023

kamilmysliwiec added a commit that referenced this issue Feb 8, 2023

Merge pull request #11073 from H4ad/fix/issue-with-xxhash

a64b9cc

fix(core): removed xxhash to fix #11071

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

"perf(core): change sha1 to xxhash" commit broke runtime of Nest.js application on Ubuntu 20.04 #11071

"perf(core): change sha1 to xxhash" commit broke runtime of Nest.js application on Ubuntu 20.04 #11071

fiki574 commented Feb 7, 2023 •

edited

Loading

micalevisk commented Feb 7, 2023 •

edited

Loading

fiki574 commented Feb 7, 2023

jmcdo29 commented Feb 7, 2023

fiki574 commented Feb 7, 2023 •

edited

Loading

H4ad commented Feb 7, 2023

kamilmysliwiec commented Feb 7, 2023

H4ad commented Feb 7, 2023

kamilmysliwiec commented Feb 8, 2023

"perf(core): change sha1 to xxhash" commit broke runtime of Nest.js application on Ubuntu 20.04 #11071

"perf(core): change sha1 to xxhash" commit broke runtime of Nest.js application on Ubuntu 20.04 #11071

Comments

fiki574 commented Feb 7, 2023 • edited Loading

Did you read the migration guide?

Is there an existing issue that is already proposing this?

Potential Commit/PR that introduced the regression

NestJS version

Describe the regression

Minimum reproduction code

Input code

Expected behavior

Other

micalevisk commented Feb 7, 2023 • edited Loading

fiki574 commented Feb 7, 2023

jmcdo29 commented Feb 7, 2023

fiki574 commented Feb 7, 2023 • edited Loading

H4ad commented Feb 7, 2023

kamilmysliwiec commented Feb 7, 2023

H4ad commented Feb 7, 2023

kamilmysliwiec commented Feb 8, 2023

fiki574 commented Feb 7, 2023 •

edited

Loading

micalevisk commented Feb 7, 2023 •

edited

Loading

fiki574 commented Feb 7, 2023 •

edited

Loading