use promises instead of callbacks, when calling mongodb, so we can upgrade it #634

adrienjoly · 2023-07-28T10:03:46Z

Now that we have upgraded mongodb driver from v3 to v4 (see #630), we should migrate callbacks to promises.

Why? As shown in #665 (comment), upgrading MongoDB 4 --> 5 cause TypeScript errors because of callbacks being passed to mongodb-driver, cf https://github.com/openwhyd/openwhyd/actions/runs/5912435427/job/16035845596#step:6:43.

For instance:

> openwhyd@1.52.15 lint:jsdoc-typing
> npx --yes tsc --target es2015 --moduleResolution node --noEmit --allowJs --skipLibCheck `find app/ -name '*.js' -name '*.d.ts'` `find test/ -name '*.js' -name '*.d.ts'`

Error: app.js(133,22): error TS2348: Value of type 'typeof MongoStore' is not callable. Did you mean to include 'new'?
Error: app/models/mongodb.js(139,17): error TS2554: Expected 0 arguments, but got 1.
Error: app/models/mongodb.js(156,27): error TS2559: Type '(err: any, collections: any) => void' has no properties in common with type 'ListCollectionsOptions'.
Error: app/models/user.js(219,[42](https://github.com/openwhyd/openwhyd/actions/runs/5912435427/job/16035845596#step:6:43)): error TS2769: No overload matches this call.

=> When all calls are fixes, let's upgrade to mongodb driver v5.

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

Follow up of PR #666. Contribute to #634. => From 219 to 209 warnings left to fix. * fix `'parse' is deprecated. Use the WHATWG URL API instead` * fix `Callbacks are deprecated` in activity model * fix `Callbacks are deprecated` in comment model * fix `Callbacks are deprecated` in follow model * make test diff more legible cf https://github.com/openwhyd/openwhyd/actions/runs/5953409421/job/16147584118?pr=676#step:6:177 * fix call to dbHandler in follow:add() cf https://github.com/openwhyd/openwhyd/actions/runs/5953409421/job/16147584118?pr=676#step:6:177 * fix test for correctness + legible diff display * reset-test-db.js: don't fail if upload directories don't exist at startup cf https://github.com/openwhyd/openwhyd/actions/runs/5953568519/job/16148115227?pr=676#step:4:19 * apply sonarcloud recommendations

Follow up of PRs #676 and #666. Contributes to #634.

Follow up of PRs #677, #676 and #666. Contributes to #634. * fix `Callbacks are deprecated` in notif model * delete deprecated mongodb options found by `@ts-check` * fix `Property 'then' does not exist on type 'FindCursor<Document>'.` * refactor: line whenDone() in forEach() * fix: move whenDone() to then() * refactor: flatten callback hell using await * fix: forEach() doesn't pass any `error` + flatten `clearUserNotifs` again * 😅 making TypeScript analysis loop infinitely when trying to type db.notif, so that $pull operation is seen as valid cf https://stackoverflow.com/a/67043340/592254 * ignore false alert on $pull from typescript checker cf https://www.mongodb.com/community/forums/t/type-objectid-is-not-assignable-to-type-never/139699 * fix deprecations + callback hell in notif model * ignore `Expected 0-1 arguments, but got 2.` ts errors * fix remaining deprecation warnings in notif model * fix `callback is not a function` error revealed by `hot-tracks` approval tests * apply suggestion from codacy * apply suggestion from sonarcloud: Prefer using an optional chain expression instead, as it's more concise and easier to read. cf https://sonarcloud.io/project/issues?resolved=false&types=CODE_SMELL&pullRequest=678&id=openwhyd_openwhyd&open=AYomPq2Mlp4D0TuBpJrq

Follow up of PRs #678, #677, #676 and #666. Contributes to #634. * fix `Callbacks are deprecated` in `track` model * fix `Type 'string[][]' is not assignable to type 'Sort'` * fix remaining callbacks passed to mongodb

adrienjoly · 2023-08-24T14:33:39Z

Note: I just had to restart openwhyd's server (db was still up and running), as it was non responsive.

Screenshot of monitoring graphs from datadog around that time:

=> suspecting some of the recent changes on MongoDB calls as a root cause for this incident.

adrienjoly · 2023-08-24T14:37:27Z

=> I just rolled back production to v1.55.19

adrienjoly · 2023-08-24T14:40:39Z

Screenshots of errors for each deployed release/version of openwhyd:

cf https://app.datadoghq.com/apm/services/openwhyd/operations/express.request/deployments?deployments=qson%3A%28data%3A%28hits%3A%28selected%3Aversion_count%29%2Cerrors%3A%28selected%3Aversion_count%29%2Clatency%3A%2195%2CtopN%3A%215%29%2Cversion%3A%210%29&env=production&summary=qson%3A%28data%3A%28visible%3A%21t%2Cerrors%3A%28selected%3Acount%29%2Chits%3A%28selected%3Acount%29%2Clatency%3A%28selected%3Alatency%2Cslot%3A%28agg%3A75%29%2Cdistribution%3A%28isLogScale%3A%21f%29%29%2Csublayer%3A%28slot%3A%28layers%3Aservice%29%2Cselected%3Apercentage%29%29%2Cversion%3A%211%29&start=1692283204137&end=1692888004137&paused=false

Contributes to #634. * fix callbacks passed to mongodb, in post model * fix counter of refs to deprecated `usernames` cache cause: some refs were probably not detected earlier, because of complicated callback logic contributes to #580. * fix(codacy): Always provide a base when using parseInt() functions * check for TypeScript errors in `post.js` * fix `Property 'nbPostsPerNewsfeedPage' does not exist on type 'typeof import("/Users/adrienjoly/Code/openwhyd/app/models/config")'` * fix deprecated mongodb options * fix `Cannot read properties of undefined (reading 'nbPostsPerNewsfeedPage')` cf https://github.com/openwhyd/openwhyd/actions/runs/5983938243/job/16234633467?pr=680#step:7:22 * add test: `incrPlayCounter` action should increase the number of plays of the track * add failing test: `incrPlayCounter` action should return the post _id * fix(api): incrPlayCounter action to not return the `post` object anymore !BREAKING * add failing test: `should increase the total number of plays of that track` => make sure that db is cleared between each test * fix test `post api should add a track to a new playlist`, to make it isolated/independant from other tests of the suite * fix test: `should increase the total number of plays of that track`, as `track` documents are created lazily * test should not pass => make `updateByEid` assert that `eId` param is defined * fix test by setting a eId * fix `incrPlayCounter` impl. by fetching the post's eId * playlist tests: prevent side effects between tests * refactor: extract callPlaylistApi * playlist tests: use DUMMY_USER instead of ADMIN_USER * playlist tests: make sure that freshly created playlist is persisted in db * add test: should rename a playlist * add failing test: should update the playlist's name in associated posts * fix: userModel.setPlaylist to await the update of posts * fix test, to match with schema of posts * be more decisive on when to cleanup, in post.api.tests.js * skip tests that cause side effects on other tests example for #684. * fix(sonar): `Promise returned in function argument where a void return was expected.` * 🙌 re-enable all playlist api tests * remove callback from setPlaylist() * add tests for playlist removal and its impact on associated posts * remove callback from unsetPlaylist() * fix some error cases, based on code review * simplify call expression * refactor: move up insertPost() and callPostApi() helpers * add tests for toggleLovePost * reveal missing callbacks in setPostLove * remove callback from setPostLove()

temporary workaround to reduce stress on mongodb, until we find a solution to solve #681 and #682. root cause may be caused/related to #630 and/or #634.

## [1.55.61](v1.55.60...v1.55.61) (2023-09-02) ### Bug Fixes * linter issues caused by previous commit ([17998a1](17998a1)) * **perf:** disable history on home page ([52536d3](52536d3)), closes [#681](#681) [#682](#682) [#630](#630) [#634](#634)

- argon2 ^0.31.0 → ^0.31.1, because we're developing an alternative: #705 - connect-mongo ^3.2.0 → ^5.0.0, because we're developing an alternative: #705 - formidable ^2.1.1 → ^3.5.1, because it breaks the build (cf #709 and #665) - mongodb 4.17.0 → 6.0.0, because we're not done yet on migrative callbacks to promises (cf #634 and #665) This partially reverts commit 43a63cf.

`$ npx npm-check-updates -u` Updates: - @applitools/eyes-cypress ^3.37.0 → ^3.38.0 - @cypress/code-coverage ^3.11.0 → ^3.12.0 - @types/node ^20.5.7 → ^20.6.0 - approvals ^6.2.1 → ^6.2.2 - cypress ^12.17.4 → ^13.1.0 - dd-trace ^4.14.0 → ^4.15.0 - eslint ^8.48.0 → ^8.49.0 Skipped updates: - argon2 ^0.31.0 → ^0.31.1, because we're developing an alternative: #705 - connect-mongo ^3.2.0 → ^5.0.0, because we're developing an alternative: #705 - formidable ^2.1.1 → ^3.5.1, because it breaks the build (cf #709 and #665) - mongodb 4.17.0 → 6.0.0, because we're not done yet on migrative callbacks to promises (cf #634 and #665)

## What does this PR do / solve? Whenever a db error happens during the generation of a notification digest email, the `notifEmails` worker hangs, causing a loop of `[OneAtATime] fct is trying to start before last call has ended` errors. <img width="1185" alt="image" src="https://github.com/openwhyd/openwhyd/assets/531781/5ad3533b-2e38-4d90-8ba0-566f47049cb7"> The problem was probably revealed while migrating on #634, because of not calling "`cb`" callback functions in case of errors returned by `async` mongodb calls. ## Overview of changes - Intercept errors, so the execution of the worker can end and be restarted later in case of unexpected interruption. - => turn callback-based functions into `async` functions, so that errors can be intercepted.

## [1.58.3](v1.58.2...v1.58.3) (2023-11-04) ### Bug Fixes * **digest:** end worker process in case of error ([#752](#752)) ([39ad323](39ad323)), closes [#634](#634)

adrienjoly mentioned this issue Jul 28, 2023

Upgrade MongoDB server to version ≥3.6, so we can upgrade its Node.js driver #630

Closed

adrienjoly self-assigned this Jul 28, 2023

adrienjoly mentioned this issue Jul 28, 2023

Upgrade calls to insert() and update(), to match with Mongodb driver ~~v2~~ v4's API #62

Open

13 tasks

adrienjoly mentioned this issue Aug 23, 2023

fix: eslint warnings #676

Merged

adrienjoly mentioned this issue Aug 24, 2023

fix: Callbacks are deprecated eslint warnings in mongodb model #677

Merged

adrienjoly added a commit that referenced this issue Aug 24, 2023

fix: Callbacks are deprecated eslint warnings in mongodb model

aae30b5

Follow up of PRs #676 and #666. Contributes to #634.

adrienjoly mentioned this issue Aug 24, 2023

fix: eslint warnings in notif model #678

Merged

adrienjoly mentioned this issue Aug 24, 2023

fix(deps): upgrade all dependencies, except those that break the build #665

Merged

adrienjoly added enhancement dependencies Pull requests that update a dependency file labels Aug 24, 2023

adrienjoly changed the title ~~use promises instead of callbacks, for mongodb calls~~ use promises instead of callbacks, when calling mongodb, so we can upgrade it Aug 24, 2023

adrienjoly mentioned this issue Aug 24, 2023

fix: deprecated mongodb calls in track model #679

Merged

adrienjoly mentioned this issue Aug 24, 2023

fix: deprecated callbacks passed to mongodb, in post model #680

Merged

adrienjoly added a commit that referenced this issue Sep 2, 2023

fix(perf): disable history on home page

52536d3

temporary workaround to reduce stress on mongodb, until we find a solution to solve #681 and #682. root cause may be caused/related to #630 and/or #634.

adrienjoly mentioned this issue Sep 11, 2023

fix(deps): update most dependencies #729

Merged

adrienjoly mentioned this issue Nov 4, 2023

fix: end worker process in case of error #752

Merged

adrienjoly pushed a commit that referenced this issue Nov 4, 2023

chore(release): 1.58.3 [skip ci]

03fc078

## [1.58.3](v1.58.2...v1.58.3) (2023-11-04) ### Bug Fixes * **digest:** end worker process in case of error ([#752](#752)) ([39ad323](39ad323)), closes [#634](#634)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

use promises instead of callbacks, when calling mongodb, so we can upgrade it #634

use promises instead of callbacks, when calling mongodb, so we can upgrade it #634

adrienjoly commented Jul 28, 2023 •

edited

Loading

adrienjoly commented Aug 24, 2023 •

edited

Loading

adrienjoly commented Aug 24, 2023

adrienjoly commented Aug 24, 2023

use promises instead of callbacks, when calling mongodb, so we can upgrade it #634

use promises instead of callbacks, when calling mongodb, so we can upgrade it #634

Comments

adrienjoly commented Jul 28, 2023 • edited Loading

adrienjoly commented Aug 24, 2023 • edited Loading

adrienjoly commented Aug 24, 2023

adrienjoly commented Aug 24, 2023

adrienjoly commented Jul 28, 2023 •

edited

Loading

adrienjoly commented Aug 24, 2023 •

edited

Loading