fix(hook): maintain CLS context in after hooks
#9473
Conversation
encapsulate promises returned by `retry-as-promised` with `Promise.resolve` to maintain CLS context fix sequelize#9472
|
Hmm so you are saying this can also fix #9071, Can you add a test for this with other cls tests? |
|
@sushantdhiman Ok, I'll give it a try |
…ly patched Promises returned by sequelize.query must be correctly patched to maintain CLS context
|
@sushantdhiman I added a simple test case which fails in current master and passes with my PR. |
test/integration/cls.test.js
Outdated
| }); | ||
|
|
||
| it('promises returned by sequelize.query are correctly patched', function() { | ||
| const self = this; |
| expect(Sequelize._cls).to.equal(this.ns); | ||
| }); | ||
|
|
||
| it('promises returned by sequelize.query are correctly patched', function() { |
There was a problem hiding this comment.
Needs a real world test, create row under transaction, try to access that row in nested context assuming cls context is passed
There was a problem hiding this comment.
Or may be take inspiration from #9383 (2nd test looks neat)
There was a problem hiding this comment.
Actually it's already almost exactly the 2nd test from #9383.
I just removed the first two .then(), which are useless because the first sequelize.query() already returns a non-patched promise.
Pull Request check-list
Please make sure to review and check all of these items:
npm run testornpm run test-DIALECTpass with this change (including linting)?Description of change
Encapsulate promises returned by
retry-as-promisedwithPromise.resolveto maintain CLS context inafterXXXhooks.fix #9472
fix #9071