Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Cache mongo client, not client promise #1

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

Merged

Conversation

Copy link
Contributor

@baileympearson baileympearson commented Jun 6, 2024

Caching the client's promise can lead to unhandled promise rejections if the client fails to connect. Caching the client avoids this pitfall.

The mongo client will auto connect on the first operation.

Comment on lines +13 to +14
await client.connect();
// `await client.connect()` will use the default database passed in the MONGODB_URI
Copy link

@nbbeeken nbbeeken Jun 6, 2024

Choose a reason for hiding this comment

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

I am not sure what it means for connect to "use a database" and the line below still references "clientPromise".

I would uncomment the sample find code, just cutting to the chase so to speak.


if (process.env.NODE_ENV === "development") {
// In development mode, use a global variable so that the value
// is preserved across module reloads caused by HMR (Hot Module Replacement).
let globalWithMongo = global as typeof globalThis & {
_mongoClientPromise?: Promise<MongoClient>;
_mongoClient?: MongoClient;
Copy link

@nbbeeken nbbeeken Jun 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

types/mongodb.d.ts still references the old global.

Optional, can we use globalThis (instead of global) here? Sticks out as legacy to me, but not necessary.

Copy link

Thanks for this guys! Curious to know if you have a solution for people using prisma as ORM with the mongodb driver?

Copy link

Thanks applied the same solution. Will confirm how it goes after observing for a week or so

Copy link
Member

@joellord joellord left a comment

Choose a reason for hiding this comment

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

I tested everything locally and it works. I did not have the 504 errors though, so I can't confirm that it solves that issue. It does not, however, introduce new errors.

@joellord joellord merged commit 6a52d8e into mongodb-developer:main Jun 7, 2024
Copy link

I tested everything locally and it works. I did not have the 504 errors though, so I can't confirm that it solves that issue. It does not, however, introduce new errors.

This fix works very well. I have been observing vercel logs for the last 10 hrs for my app and not seeing 500 errors anymore.
afwcole reacted with thumbs down emoji alexbevi and suitux reacted with hooray emoji

Copy link
Contributor Author

Hey @Rieranthony - Prisma doesn't actually use the Node driver (see prisma/prisma#12886). If you're seeing timeout errors from Prisma, I'd suggest reaching out to them, since they'll be better equipped to troubleshoot.

Copy link

I just updated my code because of the same pattern. Hopefully, this will fix the troublemaker!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joellord joellord joellord approved these changes

+1 more reviewer

@nbbeeken nbbeeken nbbeeken requested changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /