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

ODAP-Hermes (SATP) package stopped working #2984

Closed
RafaelAPB opened this issue Jan 18, 2024 · 7 comments · Fixed by #3011 or #3324
Closed

ODAP-Hermes (SATP) package stopped working #2984

RafaelAPB opened this issue Jan 18, 2024 · 7 comments · Fixed by #3011 or #3324
Labels
bug Something isn't working

Comments

@RafaelAPB
Copy link
Contributor

RafaelAPB commented Jan 18, 2024

Describe the bug

All the tests of the ODAP-Hermes business logic plugin were passing flawlessly. @AndreAugusto11 try to execute them in different environments and failed.

To Reproduce

Execute one of the integration tests. For example, client-crash-after-delete-asset.test.ts:
npx jest packages/cactus-plugin-odap-hermes/src/test/typescript/integration/client-crash-after-delete-asset.test.ts.

Expected behavior

Test passes

Logs/Stack traces

Test emits seg. fault in l. 160, where we are importing kuboRpcModule:
``const kuboRpcModule = await import("kubo-rpc-client") .

Was there any project-level configuration change that made invalid such import?

Logs provided by @AndreAugusto11 (same result for me): https://gist.github.com/AndreAugusto11/3ec7cc17087edd4d17b0646cc2bdd3ba

Operating system name, version, build:
Node: v16.20.2
Npm: v8.19.4
Commit: 1fb2551 (latest)

"(uname -srm)\n$(cat /etc/os-release)\n"
bash: Linux 5.15.0-1048-gcp x86_64\nNAME="Ubuntu"
VERSION="20.04.6 LTS (Focal Fossa)"
ID=ubuntu
ID_LIKE=debian
PRETTY_NAME="Ubuntu 20.04.6 LTS"
VERSION_ID="20.04"
HOME_URL="https://www.ubuntu.com/"
SUPPORT_URL="https://help.ubuntu.com/"
BUG_REPORT_URL="https://bugs.launchpad.net/ubuntu/"
PRIVACY_POLICY_URL="https://www.ubuntu.com/legal/terms-and-policies/privacy-policy"
VERSION_CODENAME=focal
UBUNTU_CODENAME=focal\n: No such file or directory

Also tested with node v18.19 and npm v10.2.3.


**Hyperledger Cactus release version or commit (git rev-parse --short HEAD):**
commit 1fb2551a (22 December 2023)

@RafaelAPB RafaelAPB added the bug Something isn't working label Jan 18, 2024
@AndreAugusto11
Copy link
Contributor

Adding another log for another ODAP-related bug. In this instance, the CBDC app was run using npm run start:example-cbdc-bridging-app: https://gist.github.com/AndreAugusto11/83de1325af47e77d67cf1500b00599a8

The error:

CbdcBridgingApp crashed. Existing... Error: Cannot find module '../../../../knex/knexfile.ts'
Require stack:
- /home/andre_augusto/cacti/packages/cactus-plugin-odap-hermes/dist/lib/main/typescript/gateway/plugin-odap-gateway.js
- /home/andre_augusto/cacti/packages/cactus-plugin-odap-hermes/dist/lib/main/typescript/public-api.js
- /home/andre_augusto/cacti/packages/cactus-plugin-odap-hermes/dist/lib/main/typescript/index.js
- /home/andre_augusto/cacti/examples/cactus-example-cbdc-bridging-backend/dist/lib/main/typescript/cbdc-bridging-app.js
- /home/andre_augusto/cacti/examples/cactus-example-cbdc-bridging-backend/dist/lib/main/typescript/cbdc-bridging-app-cli.js
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:1028:15)
    at Function.Module._resolveFilename.sharedData.moduleResolveFilenameHook.installedValue [as _resolveFilename] (/home/andre_augusto/cacti/node_modules/@cspotcode/source-map-support/source-map-support.js:811:30)
    at Function.Module._load (node:internal/modules/cjs/loader:873:27)
    at Module.require (node:internal/modules/cjs/loader:1100:19)
    at require (node:internal/modules/cjs/helpers:119:18)
    at FabricOdapGateway.defineKnexConnection (/home/andre_augusto/cacti/packages/cactus-plugin-odap-hermes/src/main/typescript/gateway/plugin-odap-gateway.ts:195:24)
    at new PluginOdapGateway (/home/andre_augusto/cacti/packages/cactus-plugin-odap-hermes/src/main/typescript/gateway/plugin-odap-gateway.ts:190:10)
    at new FabricOdapGateway (/home/andre_augusto/cacti/examples/cactus-example-cbdc-bridging-backend/src/main/typescript/odap-extension/fabric-odap-gateway.ts:42:5)
    at CbdcBridgingAppDummyInfrastructure.createClientGateway (/home/andre_augusto/cacti/examples/cactus-example-cbdc-bridging-backend/src/main/typescript/infrastructure/cbdc-bridging-app-dummy-infrastructure.ts:310:33)
    at CbdcBridgingApp.start (/home/andre_augusto/cacti/examples/cactus-example-cbdc-bridging-backend/src/main/typescript/cbdc-bridging-app.ts:103:57)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async launchApp (/home/andre_augusto/cacti/examples/cactus-example-cbdc-bridging-backend/src/main/typescript/cbdc-bridging-app-cli.ts:72:5) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/home/andre_augusto/cacti/packages/cactus-plugin-odap-hermes/dist/lib/main/typescript/gateway/plugin-odap-gateway.js',
    '/home/andre_augusto/cacti/packages/cactus-plugin-odap-hermes/dist/lib/main/typescript/public-api.js',
    '/home/andre_augusto/cacti/packages/cactus-plugin-odap-hermes/dist/lib/main/typescript/index.js',
    '/home/andre_augusto/cacti/examples/cactus-example-cbdc-bridging-backend/dist/lib/main/typescript/cbdc-bridging-app.js',
    '/home/andre_augusto/cacti/examples/cactus-example-cbdc-bridging-backend/dist/lib/main/typescript/cbdc-bridging-app-cli.js'
  ]
}

@outSH
Copy link
Contributor

outSH commented Jan 18, 2024

@AndreAugusto11 This error can be solved by going one level higher when looking for knex in file packages/cactus-plugin-odap-hermes/src/main/typescript/gateway/plugin-odap-gateway.ts:

    const configFile = require("../../../../knex/knexfile.ts")[
      process.env.ENVIRONMENT || "development"
    ];

to

    const configFile = require("../../../../../knex/knexfile.ts")[
      process.env.ENVIRONMENT || "development"
    ];

Caught this issue some time ago but did not have time to do follow up work on it, sorry :/ In general importing local files like this is very error prone (as we can see), so I'd replace it with env variable. It's possible it used to work before changing module resolution to Node16.

@outSH
Copy link
Contributor

outSH commented Jan 18, 2024

@RafaelAPB Yeah, ipfs-http-client was replaced with kubo-rpc-client because the former is deprecated. kubo-rpc-client on the other hand comes as ESM module only, so we had to use async import (since we still use older CJS modules). And finally async import doesnt seem fix the problem in jest. The best solution is to migrate to ESM and hope that some of these issues goes await, but I think we're out of working capacity at the moment (although we'd hope to have it done before V2). Alternatively we can try to self-release CJS version of this module like @petermetz did with some other dependencies.

@AndreAugusto11
Copy link
Contributor

AndreAugusto11 commented Jan 18, 2024

@AndreAugusto11 This error can be solved by going one level higher when looking for knex in file packages/cactus-plugin-odap-hermes/src/main/typescript/gateway/plugin-odap-gateway.ts:

    const configFile = require("../../../../knex/knexfile.ts")[
      process.env.ENVIRONMENT || "development"
    ];

to

    const configFile = require("../../../../../knex/knexfile.ts")[
      process.env.ENVIRONMENT || "development"
    ];

Caught this issue some time ago but did not have time to do follow up work on it, sorry :/ In general importing local files like this is very error prone (as we can see), so I'd replace it with env variable. It's possible it used to work before changing module resolution to Node16.

Hi @outSH, thanks for the help! It's probably because we should account for the file being run inside the dist/ folder.
Given that the content is dynamic, do you have ideas on how to do this instead of using env variables (I'm assuming it will not work)? The content of the knexfile.ts is:

module.exports = {
  development: {
    client: "sqlite3",
    connection: {
      filename: path.resolve(__dirname, ".dev-" + uuidv4() + ".sqlite3"),
    },
    migrations: {
      directory: path.resolve(__dirname, "migrations"),
    },
    useNullAsDefault: true,
  },
};

@RafaelAPB
Copy link
Contributor Author

RafaelAPB commented Jan 23, 2024

@outSH thanks a lot. We will try to fix this ourselves. If you have some more details, please share them with us (for example, can you point us to this other dependencies where a CJS self-release was done).

@outSH
Copy link
Contributor

outSH commented Jan 30, 2024

Hi @outSH, thanks for the help! It's probably because we should account for the file being run inside the dist/ folder.
Given that the content is dynamic, do you have ideas on how to do this instead of using env variables (I'm assuming it will not work)? The content of the knexfile.ts is:

@AndreAugusto11 Sorry for late answer. I think that the DB file and migrations dir should be configurable in some way (either from envvar or through plugin constructor / setup), if we do that then this file is no longer dynamic and could be included in the source of the file. If for some reason this can't be done, then I think you can add a postbuild script that will copy all the needed files to dist (example from another package - https://github.com/hyperledger/cacti/blob/main/packages/cactus-plugin-ledger-connector-ethereum/package.json#L55)

@outSH
Copy link
Contributor

outSH commented Jan 30, 2024

@outSH thanks a lot. We will try to fix this ourselves. If you have some more details, please share them with us (for example, can you point us to this other dependencies where a CJS self-release was done).

@RafaelAPB Details are in this PR (see peter additional changes part): https://github.com/hyperledger/cacti/pull/2829

AndreAugusto11 added a commit to AndreAugusto11/cacti that referenced this issue Feb 2, 2024
* implement the repository design pattern to make storage technology agnostic
* due to the deprecation of the ipfs package, this allows one to choose another storage
* refactoring of the tests and the CBDC example that is based on the SATP
* implement the remote log storage as a sqlite database
* add post build instruction to copy knex files to dist/

closes hyperledger-cacti#2984

Signed-off-by: André Augusto <[email protected]>
AndreAugusto11 referenced this issue in RafaelAPB/blockchain-integration-framework Feb 2, 2024
* implement the repository design pattern to make storage technology agnostic
* due to the deprecation of the ipfs package, this allows one to choose another storage
* refactoring of the tests and the CBDC example that is based on the SATP
* implement the remote log storage as a sqlite database
* add post build instruction to copy knex files to dist/

closes hyperledger#2984

Signed-off-by: André Augusto <[email protected]>
AndreAugusto11 added a commit to AndreAugusto11/cacti that referenced this issue Feb 6, 2024
* implement the repository design pattern to make storage technology agnostic
* due to the deprecation of the ipfs package, this allows one to choose another storage
* refactoring of the tests and the CBDC example that is based on the SATP
* implement the remote log storage as a sqlite database
* add post build instruction to copy knex files to dist/

closes hyperledger-cacti#2984

Signed-off-by: André Augusto <[email protected]>
AndreAugusto11 added a commit to AndreAugusto11/cacti that referenced this issue Feb 7, 2024
* implement the repository design pattern to make storage technology agnostic
* due to the deprecation of the ipfs package, this allows one to choose another storage
* refactoring of the tests and the CBDC example that is based on the SATP
* implement the remote log storage as a sqlite database
* add post build instruction to copy knex files to dist/

closes hyperledger-cacti#2984

Signed-off-by: André Augusto <[email protected]>
AndreAugusto11 added a commit to AndreAugusto11/cacti that referenced this issue Feb 7, 2024
* implement the repository design pattern to make storage technology agnostic
* due to the deprecation of the ipfs package, this allows one to choose another storage
* refactoring of the tests and the CBDC example that is based on the SATP
* implement the remote log storage as a sqlite database
* add post build instruction to copy knex files to dist/

closes hyperledger-cacti#2984

Signed-off-by: André Augusto <[email protected]>
AndreAugusto11 added a commit to AndreAugusto11/cacti that referenced this issue Feb 7, 2024
* implement the repository design pattern to make storage technology agnostic
* due to the deprecation of the ipfs package, this allows one to choose another storage
* refactoring of the tests and the CBDC example that is based on the SATP
* implement the remote log storage as a sqlite database
* add post build instruction to copy knex files to dist/

closes hyperledger-cacti#2984

Signed-off-by: André Augusto <[email protected]>
AndreAugusto11 added a commit to AndreAugusto11/cacti that referenced this issue Feb 7, 2024
* implement the repository design pattern to make storage technology agnostic
* due to the deprecation of the ipfs package, this allows one to choose another storage
* refactoring of the tests and the CBDC example that is based on the SATP
* implement the remote log storage as a sqlite database
* add post build instruction to copy knex files to dist/

closes hyperledger-cacti#2984

Signed-off-by: André Augusto <[email protected]>
AndreAugusto11 added a commit to AndreAugusto11/cacti that referenced this issue Feb 7, 2024
*implement the repository design pattern to make storage technology-agnostic
* due to the deprecation of the ipfs package, this allows one to choose another storage
* refactoring of the tests and the CBDC example that is based on the SATP
* implement the remote log storage as a SQLite database
* add post-build instruction to copy knex files to dist/

closes hyperledger-cacti#2984

depends on hyperledger-cacti#3006

Signed-off-by: André Augusto <[email protected]>
AndreAugusto11 added a commit to AndreAugusto11/cacti that referenced this issue Feb 7, 2024
*implement the repository design pattern to make storage technology-agnostic
* due to the deprecation of the ipfs package, this allows one to choose another storage
* refactoring of the tests and the CBDC example that is based on the SATP
* implement the remote log storage as a SQLite database
* add post-build instruction to copy knex files to dist/

closes hyperledger-cacti#2984

depends on hyperledger-cacti#3006

Signed-off-by: André Augusto <[email protected]>
AndreAugusto11 added a commit to AndreAugusto11/cacti that referenced this issue Feb 7, 2024
*implement the repository design pattern to make storage technology-agnostic
* due to the deprecation of the ipfs package, this allows one to choose another storage
* refactoring of the tests and the CBDC example that is based on the SATP
* implement the remote log storage as a SQLite database
* add post-build instruction to copy knex files to dist/

closes hyperledger-cacti#2984

depends on hyperledger-cacti#3006

Signed-off-by: André Augusto <[email protected]>
petermetz pushed a commit that referenced this issue Feb 7, 2024
*implement the repository design pattern to make storage technology-agnostic
* due to the deprecation of the ipfs package, this allows one to choose another storage
* refactoring of the tests and the CBDC example that is based on the SATP
* implement the remote log storage as a SQLite database
* add post-build instruction to copy knex files to dist/

closes #2984

depends on #3006

Signed-off-by: André Augusto <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants