kurierjs / kurier

TypeScript framework to create JSON:API compliant APIs
https://kurier.readthedocs.io/en/latest/
MIT License
61 stars 9 forks source link

Updates all dependencies (a follow up to #271) #279

Closed joelalejandro closed 3 years ago

joelalejandro commented 3 years ago

After three long months of delaying work on upgrading dependencies, I finally got around to the problems @spersico found in #271.

Older versions of Knex didn't support (or probably, it did support it but we didn't notice) fully async transaction creation. Thus, we wrapped knex.transaction() in a promise to auto-resolve it, making it compatible with async/await.

   return new Promise(resolve =>
     knex.transaction((trx: Knex.Transaction) => {
       resolve(trx);
     })
   );

With the current version, documentation reports that we can directly use:

   return knex.transaction();

Using trace-unhandled with the dummy-app, I discovered we had a number of UnhandledPromiseRejection errors on every call:

(node:1225) UnhandledPromiseRejectionWarning
[ Stacktrace altered by https://github.com/grantila/trace-unhandled ]
Error: 404: not_found
    ==== Promise at: ==================
    at Promise.then (<anonymous>)
    at new Transaction (/home/joey/dev/kurier/node_modules/knex/lib/execution/transaction.js:84:33)
    at new Transaction_Sqlite (/home/joey/dev/kurier/node_modules/knex/lib/dialects/sqlite3/execution/sqlite-transaction.js:3:1)
    at Client_SQLite3.transaction (/home/joey/dev/kurier/node_modules/knex/lib/dialects/sqlite3/index.js:39:12)
    at Object._transaction (/home/joey/dev/kurier/node_modules/knex/lib/knex-builder/make-knex.js:146:33)
    at Object.transaction (/home/joey/dev/kurier/node_modules/knex/lib/knex-builder/make-knex.js:139:19)
    at Function.value (/home/joey/dev/kurier/node_modules/knex/lib/knex-builder/make-knex.js:91:29)
    at /home/joey/dev/kurier/src/application.ts:158:12
    at new TraceablePromise (/home/joey/dev/kurier/node_modules/trace-unhandled/dist/lib/core.js:81:13)
    at Application.createTransaction (/home/joey/dev/kurier/src/application.ts:157:12)
    at Application.executeOperations (/home/joey/dev/kurier/src/application.ts:57:50)
    at Object.handleJsonApiEndpoint (/home/joey/dev/kurier/src/utils/http-utils.ts:64:65)
    at jsonApiKoa (/home/joey/dev/kurier/src/middlewares/json-api-koa.ts:34:38)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)

    ==== Error at: ====================
    at Object.RecordNotExists (/home/joey/dev/kurier/src/errors/json-api-errors.ts:8:41)
    at ArticleProcessor.get (/home/joey/dev/kurier/src/processors/knex-processor.ts:161:27)
    at ArticleProcessor.execute (/home/joey/dev/kurier/src/processors/operation-processor.ts:34:37)
    at Application.executeOperation (/home/joey/dev/kurier/src/application.ts:142:20)
    at async Promise.all (index 0)
    at Application.executeOperations (/home/joey/dev/kurier/src/application.ts:60:43)
    at Object.handleJsonApiEndpoint (/home/joey/dev/kurier/src/utils/http-utils.ts:64:43)
    at jsonApiKoa (/home/joey/dev/kurier/src/middlewares/json-api-koa.ts:34:32)

    ==== Shared trace: ================

This particular piece of the stack trace got me in the right direction:

    at new TraceablePromise (/home/joey/dev/kurier/node_modules/trace-unhandled/dist/lib/core.js:81:13)
    at Application.createTransaction (/home/joey/dev/kurier/src/application.ts:157:12)

So, this is it. The PR that fixes all things (TM).

joelalejandro commented 3 years ago

@spersico For some reason CI is not passing, apparently there's a deep dependency package conflict, related to Node versions:

image

Can you take a look?

Fixed, had to update a CI action.

joelalejandro commented 3 years ago

@spersico This is good to go!