fix(adapter-mongodb): Improve MongoDB connection handling for serverless environments

[simonecervini]

☕️ Reasoning

This PR, opened as requested by @ndom91, revises and builds on the work from #8753.

It integrates the new changes into the existing MongoDB adapter.

🧢 Checklist

• Documentation
• Tests
• Ready to be merged

🎫 Affected issues

Fixes: #8411

📌 Resources

• Security guidelines
• Contributing guidelines
• Code of conduct
• Contributing to Open Source

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
auth-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 1, 2024 10:29am

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
next-auth-docs	⬜️ Ignored (Inspect)	Visit Preview		Jun 1, 2024 10:29am

[vercel]

@simonecervini is attempting to deploy a commit to the authjs Team on Vercel.

A member of the Team first needs to authorize it.

[balazsorban44]

Looks like the perfect usecase for the new using keyword in TypeScript! Would you like to give it a try?

https://www.typescriptlang.org/docs/handbook/release-notes/typescript-5-2.html#using-declarations-and-explicit-resource-management

[simonecervini]

Looks like the perfect usecase for the new using keyword in TypeScript! Would you like to give it a try?

https://www.typescriptlang.org/docs/handbook/release-notes/typescript-5-2.html#using-declarations-and-explicit-resource-management

@balazsorban44 I've already tried it, but there are some issues with prettier when you do something like this:

await using db = await getConnection();

This was fixed with prettier/prettier#15321, so we need to upgrade prettier first.

[balazsorban44]

Prettier bumped! 👍

[simonecervini]

Thank you @balazsorban44! I refactored the code to use using, but the tests fail because of TypeError: Property [Symbol.dispose] is not a function. I looked at the transpiled code, here's the playground with the correct version of swc used by jest.

It looks like Symbol.asyncDispose is undefined in the test environment, maybe we need a polyfill.

EDIT: I can confirm this, with this diff all the tests pass

diff --git a/packages/adapter-mongodb/package.json b/packages/adapter-mongodb/package.json
index 93a3e71b..928a6a8f 100644
--- a/packages/adapter-mongodb/package.json
+++ b/packages/adapter-mongodb/package.json
@@ -45,7 +45,8 @@
     "mongodb": "^6"
   },
   "devDependencies": {
-    "mongodb": "^6.0.0"
+    "mongodb": "^6.0.0",
+    "@hazae41/symbol-dispose-polyfill": "latest"
   },
   "jest": {
     "preset": "utils/adapter"
diff --git a/packages/adapter-mongodb/tests/custom.test.ts b/packages/adapter-mongodb/tests/custom.test.ts
index acfe357f..46341b63 100644
--- a/packages/adapter-mongodb/tests/custom.test.ts
+++ b/packages/adapter-mongodb/tests/custom.test.ts
@@ -1,3 +1,5 @@
+import "@hazae41/symbol-dispose-polyfill"
+
 import { runBasicTests } from "utils/adapter"
 import { defaultCollections, format, MongoDBAdapter, _id } from "../src"
 import { MongoClient } from "mongodb"
diff --git a/packages/adapter-mongodb/tests/index.test.ts b/packages/adapter-mongodb/tests/index.test.ts
index 0dc84820..36882b99 100644
--- a/packages/adapter-mongodb/tests/index.test.ts
+++ b/packages/adapter-mongodb/tests/index.test.ts
@@ -1,3 +1,4 @@
+import "@hazae41/symbol-dispose-polyfill"
 import { runBasicTests } from "utils/adapter"
 import { defaultCollections, format, MongoDBAdapter, _id } from "../src"
 import { MongoClient } from "mongodb"

Actually, the one-liner polyfill Symbol.asyncDispose ??= Symbol("Symbol.asyncDispose") is enough to fix the tests, but the problem could be larger because most runtimes are probably not supporting this symbol natively. Should we polyfill or just wait?

[tylerstanczak]

I'm getting this error with this version of the adapter:

[auth][error] AdapterError: Read more at https://errors.authjs.dev#adaptererror
[auth][cause]: MongoNotConnectedError: Client must be connected before running operations

I have this issue with the published version too anyway though.

Is it not a better idea to create the client with the MONGODB_URI ENV instead of passing in a Promise?

[ndom91]

I'm getting this error with this version of the adapter:

[auth][error] AdapterError: Read more at https://errors.authjs.dev#adaptererror [auth][cause]: MongoNotConnectedError: Client must be connected before running operations

I have this issue with the published version too anyway though.

Is it not a better idea to create the client with the MONGODB_URI ENV instead of passing in a Promise?

Can you give this antoher shot? Specifically check out the new test that was added here if you want to see an implementation example 🙏

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 38.80%. Comparing base (77b20d5) to head (166c5bf).
Report is 3 commits behind head on main.

❗ Current head 166c5bf differs from pull request most recent head b747557

Please upload reports for the commit b747557 to get more accurate results.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9459      +/-   ##
==========================================
- Coverage   40.85%   38.80%   -2.06%     
==========================================
  Files         176      176              
  Lines       27893    27914      +21     
  Branches     1242     1224      -18     
==========================================
- Hits        11397    10831     -566     
- Misses      16496    17083     +587     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ndom91]

@simonecervini can you move the serverless.test.ts file into the test directory with the other tests?

Also I'm having trouble with the a few of the tests. First, that onClose count test at the bottom of serverless.test.ts seems to always be 0, which while matching the allClients.length, doesn't seem to be what we want. Second, the getUserByEmail is oddly failing for me in the index.test.ts suite only 🤔

Anyway, after we clear that up I think it'll be good to go!

[simonecervini]

It was probably related to #9818

It should be fixed now

[ndom91]

Looks good, thanks!