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

fix(model): support session: null option for save() to opt out of automatic session option with transactionAsyncLocalStorage #14744

Merged
merged 5 commits into from
Jul 15, 2024

Conversation

vkarpov15
Copy link
Collaborator

Fix #14736

Summary

doc.save({ session: null }) should override default session option from transactionAsyncLocalStorage. That's how await TestModel.find().setOptions({ session: null }) and others work, it looks like save() is the only one affected

Examples

… automatic `session` option with `transactionAsyncLocalStorage`

Fix #14736
Copy link
Collaborator

@hasezoey hasezoey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, should this also be added to #14742 before it is merged?

Copy link
Collaborator

@hasezoey hasezoey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually now looking at the Replica set tests, they are consistently failing with this PR:

test output snippit for reference
  5 failing
  1) transactions
       basic example:
      AssertionError [ERR_ASSERTION]: The expression evaluated to a falsy value:
  assert.ok(!doc)

      + expected - actual
      -false
      +true
      
      at /home/runner/work/mongoose/mongoose/test/docs/transactions.test.js:62:26
      at runMicrotasks (<anonymous>)
      at processTicksAndRejections (node:internal/process/task_queues:96:5)

  2) transactions
       abort:
      AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
2 !== 0

      + expected - actual
      -2
      +0
      
      at /home/runner/work/mongoose/mongoose/test/docs/transactions.test.js:110:28
      at runMicrotasks (<anonymous>)
      at processTicksAndRejections (node:internal/process/task_queues:96:5)

  3) transactions
       transaction() resets $isNew on error:
      AssertionError [ERR_ASSERTION]: The expression evaluated to a falsy value:
  assert.ok(!exists)

      + expected - actual
      -false
      +true
      
      at Context.<anonymous> (test/docs/transactions.test.js:432:12)
      at runMicrotasks (<anonymous>)
      at processTicksAndRejections (node:internal/process/task_queues:96:5)

  4) transactions
       transaction() resets $isNew between retries (gh-13698):
     MongoServerError: E11000 duplicate key error collection: mongoose_test.Test index: _id_ dup key: { _id: ObjectId('669029c2fa5898976ba20094') }
      at InsertOneOperation.execute (node_modules/mongodb/lib/operations/insert.js:51:19)
      at runMicrotasks (<anonymous>)
      at processTicksAndRejections (node:internal/process/task_queues:96:5)
      at async executeOperation (node_modules/mongodb/lib/operations/execute_operation.js:136:16)
      at async Collection.insertOne (node_modules/mongodb/lib/collection.js:155:16)

  5) transactions
       populate (gh-6754)
         `Document#populate()` supports overwriting the session:
      AssertionError [ERR_ASSERTION]: The expression evaluated to a falsy value:
  assert.ok(!article.author)

      + expected - actual
      -false
      +true
      
      at /home/runner/work/mongoose/mongoose/test/docs/transactions.test.js:287:32
      at runMicrotasks (<anonymous>)
      at processTicksAndRejections (node:internal/process/task_queues:96:5)

EDIT:
after some quick investigation, could it be that options.session is just never transferred to saveOptions.session? though i dont understand how it worked before then.

@vkarpov15
Copy link
Collaborator Author

@hasezoey the save() function creates saveOptions based on options here:

mongoose/lib/model.js

Lines 558 to 560 in 70e8e19

if (options.hasOwnProperty('session')) {
this.$session(options.session);
}

@vkarpov15 vkarpov15 added this to the 8.5.2 milestone Jul 15, 2024
@vkarpov15 vkarpov15 merged commit 93ebbe1 into master Jul 15, 2024
46 checks passed
vkarpov15 added a commit that referenced this pull request Jul 15, 2024
…session option with transactionAsyncLocalStorage; backport #14744
@hasezoey hasezoey deleted the vkarpov15/gh-14736 branch July 15, 2024 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for non-transactional operations in asynclocalstorage transactions
2 participants