josephdadams / TallyArbiter

The flexible and customizable camera tally light system
http://www.tallyarbiter.com
MIT License
286 stars 101 forks source link

[Bug]: Docker Image v3.0.4 does not start: Cannot find module bcrypt #603

Closed robo-w closed 9 months ago

robo-w commented 1 year ago

What happened?

Steps to reproduce:

Version

3.0.4

Distribution

Docker

OS

Linux

What browsers are you seeing the problem on?

No response

If applicable, What Listener Clients are You Using?

No response

TallyArbiter configuration

No response

Relevant log output

No response

Error stacktrace (if applicable)

Error: Cannot find module '/app/node_modules/bcrypt/lib/binding/napi-v3/bcrypt_lib.node'
Require stack:
- /app/node_modules/bcrypt/bcrypt.js
- /app/_helpers/auth.js
- /app/_helpers/config.js
- /app/index.js
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:933:15)
    at Function.Module._load (node:internal/modules/cjs/loader:778:27)
    at Module.require (node:internal/modules/cjs/loader:1005:19)
    at require (node:internal/modules/cjs/helpers:102:18)
    at Object.<anonymous> (/app/node_modules/bcrypt/bcrypt.js:6:16)
    at Module._compile (node:internal/modules/cjs/loader:1103:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1157:10)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Module.require (node:internal/modules/cjs/loader:1005:19) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/app/node_modules/bcrypt/bcrypt.js',
    '/app/_helpers/auth.js',
    '/app/_helpers/config.js',
    '/app/index.js'
  ]
}
github-actions[bot] commented 1 year ago

Hello there robo-w 👋

Welcome to TallyArbiter!

Thank you for opening your first issue for the Tally Arbiter project. Tally Arbiter fosters an open and welcoming environment for all our contributors. Please adhere to our Code Of Conduct.

If you have more to contribute to this issue, please comment down below! We will try to get back to you as soon as we can.

BenJamesAndo commented 11 months ago

I believe this has been fixed, but since no new releases have come out since the fix, then it's still not working. See discussion https://github.com/josephdadams/TallyArbiter/discussions/463

Jocke4f commented 10 months ago

Does it work with 3.0.5?

robo-w commented 10 months ago

I believe this has been fixed, but since no new releases have come out since the fix, then it's still not working. See discussion #463

I tried it on 26th Oct with a self-built Docker image from master branch, there it still did not work. I did not try it in the meantime. As a workaround I ran the software in development mode directly from the copied Git repo. I will try to check it again.

BenJamesAndo commented 10 months ago

v3.0.6 contains release binaries but the docker image doesn't seem to have been updated on docker hub. npm seemed to update fine.

josephdadams commented 10 months ago

v3.0.7 will have a docker image.

BenJamesAndo commented 10 months ago

@josephdadams FYI v3.0.7 isn't showing up in Docker.

josephdadams commented 10 months ago

Yeah the docker build failed. Idk why

josephdadams commented 10 months ago

https://github.com/josephdadams/TallyArbiter/actions/runs/7451847271/job/20273826348

If you want to look into it and fix it

Jocke4f commented 10 months ago

I have no idea what could be causing this, keep this in mind when continue reading this comment...

I noticed that in the workfile within the "build_docker" section sometimes specific versions are stated in the "uses: "-part. For some is it quite old. Can this be causing this build problem?

hrueger commented 10 months ago

If we look at the error message

 #39 321.8 npm ERR! code 1
#39 321.8 npm ERR! path /app/node_modules/electron
#39 321.8 npm ERR! command failed
#39 321.8 npm ERR! command sh -c node install.js
#39 321.8 npm ERR! HTTPError: Response code 404 (Not Found)
#39 321.8 npm ERR!     at Request._onResponseBase (/app/node_modules/@electron/get/node_modules/got/dist/source/core/index.js:913:31)
#39 321.8 npm ERR!     at Request._onResponse (/app/node_modules/@electron/get/node_modules/got/dist/source/core/index.js:948:24)
#39 321.8 npm ERR!     at ClientRequest.<anonymous> (/app/node_modules/@electron/get/node_modules/got/dist/source/core/index.js:962:23)
#39 321.8 npm ERR!     at Object.onceWrapper (node:events:629:26)
#39 321.8 npm ERR!     at ClientRequest.emit (node:events:526:35)
#39 321.8 npm ERR!     at origin.emit (/app/node_modules/@electron/get/node_modules/@szmarczak/http-timer/dist/source/index.js:43:20)
#39 321.8 npm ERR!     at HTTPParser.parserOnIncomingClient (node:_http_client:693:27)
#39 321.8 npm ERR!     at HTTPParser.parserOnHeadersComplete (node:_http_common:119:17)
#39 321.8 npm ERR!     at TLSSocket.socketOnData (node:_http_client:535:22)
#39 321.8 npm ERR!     at TLSSocket.emit (node:events:514:28)

it looks like the 404 is coming from the @electron/get postinstall script. Could it be that there just is no electron binary for that platform / architecture / libc/musl / etc? We probably don't need electron in the docker image. If that is the error, then the question is: why is it being installed?

josephdadams commented 10 months ago

I didn't write the build yaml, but it worked before...

Jocke4f commented 10 months ago

When I check the logs is it for the platform linux/ppc64le that the failure occurs for. Do we need to support this platform? For none of the other platforms did this step fail so it can not be general electron thing.

When I check https://github.com/docker-library/official-images#architectures-other-than-amd64 does it look like linux/ppc64le is not listed in the architectures officially supported by Docker. Does this mean that we could remove architecture or am I missunderstanding this?

I don't know how common linux/ppc64le is.

josephdadams commented 10 months ago

Let’s delete it and see if anyone complains

Jocke4f commented 10 months ago

I can add the deletion of it in a PR that I'm preparing that fixes some warnings and errors from build.yml and codeql-analysis.yml, unless you want the deletion in separate PR.

josephdadams commented 10 months ago

Sounds good

Jocke4f commented 9 months ago

Have we really fixed the problem with "Cannot find module bcrypt" really fixed?

When I launched TA in debug mode in Visual Studio Code for my freshly setup development environment with v3.0.7 did I encouter the same error:

> tallyarbiter@3.0.7 start
> ts-node-dev src/index.ts --dev

run-script-pkg.js:64
[INFO] 21:01:20 ts-node-dev ver. 1.1.8 (using ts-node ver. 9.1.1, typescript ver. 5.2.2)
log.js:27
Error: Cannot find module 'C:\Users\[REMOVED]\GitHub\TallyArbiter\node_modules\bcrypt\lib\binding\napi-v3\bcrypt_lib.node'
Require stack:
- C:\Users\[REMOVED]\Dokument\TallyArbiter\node_modules\bcrypt\bcrypt.js
- C:\Users\[REMOVED]\Dokument\TallyArbiter\src\_helpers\auth.ts
- C:\Users\[REMOVED]\Dokument\TallyArbiter\src\_helpers\config.ts
- C:\Users\[REMOVED]\Dokument\TallyArbiter\src\index.ts
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:1144:15)
    at Function.Module._load (node:internal/modules/cjs/loader:985:27)
    at Module.require (node:internal/modules/cjs/loader:1235:19)
    at require (node:internal/modules/helpers:176:18)
    at Object.<anonymous> (C:\Users\[REMOVED]\GitHub\TallyArbiter\node_modules\bcrypt\bcrypt.js:6:16)
    at Module._compile (node:internal/modules/cjs/loader:1376:14)
    at Module._compile (C:\Users\[REMOVED]\GitHub\TallyArbiter\node_modules\source-map-support\source-map-support.js:568:25)
    at Module._extensions..js (node:internal/modules/cjs/loader:1435:10)
    at Object.nodeDevHook [as .js] (C:\Users\[REMOVED]\GitHub\TallyArbiter\node_modules\ts-node-dev\lib\hook.js:63:13)
    at Module.load (node:internal/modules/cjs/loader:1207:32)

When I quickly researched this did I find the following which made me think that we haven't really fixed the root cause.

Bcrypt vs bcryptjs

https://stackoverflow.com/questions/66729013/why-is-importing-bcrypt-causing-a-cannot-find-module-napi-v3-bcrypt-lib-node-e

In this post they pointed to the solution described in

https://forum.serverless.com/t/after-making-a-deploy-i-test-the-function-and-show-this/8624/2

TLDR

Should we use bcryptjs instead of bcrypt or have I misunderstood this?

josephdadams commented 9 months ago

We should definitely switch. It’s just for the password stuff.