fix(2708): Better error handling in orbitdb to mitigate cascading crashes #2747
Conversation
islathehut
force-pushed
the
fix/2708-cascading-crashes
branch
from
ccb1950 to
9d255e7
Compare
| await (this.datastore as LevelDatastore).close() | ||
| try { | ||
| await (this.datastore as LevelDatastore).close() | ||
| await (this.datastore as LevelDatastore).db.close() |
There was a problem hiding this comment.
Why would we call LevelDatastore.db.close() when all the method LevelDatastore.close() does is call this.db.close()?
There was a problem hiding this comment.
I don't remember the issue but I had seen something about making sure the level db was actually open and closed because it could sometimes get stuck so I figured we would be cautious and verbose.
packages/backend/src/nest/storage/orbitDb/keyValueIndexedValidated.ts
Outdated
Show resolved
Hide resolved
| @@ -15,7 +15,9 @@ | |||
| "lint-staged": "lerna run lint-staged", | |||
| "build:auth": "cd ./3rd-party/auth && pnpm install && pnpm build", | |||
| "build:noise": "cd ./3rd-party/js-libp2p-noise && npm i && npm run build", | |||
| "bootstrap": "npm run build:auth && npm run build:noise && lerna bootstrap", | |||
| "build:orbitdb": "cd ./3rd-party/orbitdb && npm i && npm run build:dist && npm run build:debug", | |||
There was a problem hiding this comment.
I'm not familiar with orbitdbs builds, but it looks like we only have an npm script for building a debug build of orbitdb. Is that safe for production, or should we have a different process for the CD pipeline?
There was a problem hiding this comment.
This mimics what they do in production. The build command runs both but the reason I split it out like this is because it also runs jsdoc and for some reason that was breaking windows builds in CI. If you look in node_modules for a non-submodule branch the same files are in the orbitdb dist directory so we're good.
| @@ -13,7 +13,9 @@ | |||
| "webpack": "webpack --env mode=development && cp ./lib/bundle.cjs ../backend-bundle/bundle.cjs", | |||
| "webpack:prod": "webpack --env mode=production && cp ./lib/bundle.cjs ../backend-bundle/bundle.cjs", | |||
| "postinstall": "npm run applyPatches", | |||
| "applyPatches": "patch -f -p0 < ./patch/electron-fetch.patch || true && patch -f -p0 --forward --binary < ./patch/parse-duration.patch || true && patch -f -p0 --forward --binary < ./patch/parse-duration-esm.patch || true && patch -f -p0 < ./patch/ipfs-pubsub-peer-monitor.patch || true && patch -f -p0 < ./patch/itws-sink.patch || true && patch -f -p0 < ./patch/mplex.patch || true && patch -f -p0 < ./patch/upgrader.patch || true", | |||
| "applyPatches": "npm run applyPatches:it && npm run applyPatches:libp2p", | |||
There was a problem hiding this comment.
Nice to get rid of some patches. What changed to justify that?
There was a problem hiding this comment.
Most of them were invalid, pointing at files that didn't exist.
Pull Request Checklist
(Optional) Mobile checklist
Please ensure you completed the following checks if you did any changes to the mobile package: