Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Dev mode hot reload loses database connection #7811

Closed
seejamescode opened this issue Jul 9, 2019 · 22 comments
Closed

Dev mode hot reload loses database connection #7811

seejamescode opened this issue Jul 9, 2019 · 22 comments
Milestone

Comments

@seejamescode
Copy link

Bug report

Describe the bug

Loving v9! Just ran into an issue where I can’t use an existing database connection with the new API routes.

To Reproduce

mongooseConnect.js:

const mongoose = require("mongoose").set("debug", true);
const connection = {};

module.exports = async () => {
  if (connection.isConnected) {
    // Using existing database connection
    return;
  }

  // Using new database connection
  const db = await mongoose.connect("mongodb://localhost:27017/my-app",
    { useCreateIndex: true, useFindAndModify: false, useNewUrlParser: true }
  );

  connection.isConnected = db.connections[0].readyState;
};
  1. Require the above function into an API endpoint that uses the new pages/api/ feature and requires
  2. Run next dev
  3. Ping the API endpoint and see the correct response
  4. Edit mongooseConnect.js and save
  5. Ping the API endpoint again

Expected behavior

The response will be the same. Instead, I get the following error:

MongooseError: You can not `mongoose.connect()` multiple times while connected.

This is because db returns to {} with the HMR.

System information

  • OS: macOS
  • Version of Next.js: 9.0.0

Additional context

Is there a different way we should be handling serverless database connections or is this a bug?

@Timer
Copy link
Member

Timer commented Jul 9, 2019

👋 Hi! I believe this is expected unless you add custom module.hot handling to not fully replace this module instance.

I'll have one of our team members look into this to confirm, though.

@Timer Timer added this to the 9.0.x milestone Jul 9, 2019
@seejamescode
Copy link
Author

Thanks @Timer! I was able to add mongooseConnect.js to the ignore list with the next.config.js. However, that understandably does not save the dev experience when editing the api route file.

Is there any documentation for adding module.hot in Next.js? I was not able to use the condition directly in mongooseConnect.js. Do I use it in next.config.js?

@Timer
Copy link
Member

Timer commented Jul 9, 2019

@seejamescode under further review this isn't possible right now. We'll tag this as a feature request, sorry about that!

@Timer Timer modified the milestones: 9.0.x, 9.x Jul 9, 2019
@Timer Timer modified the milestones: 9.x, 9.0.x Jul 22, 2019
@Timer Timer modified the milestones: 9.0.x, backlog Aug 30, 2019
@prowebat
Copy link

prowebat commented Jan 3, 2020

@seejamescode:
i encountered a similiar problem.
easiest way was to save the database connection in a global variable:
so you could replace all instances of connection with eg. global.dbConnection:

@grubxy
Copy link

grubxy commented Mar 28, 2020

@seejamescode:
i encountered a similiar problem.
easiest way was to save the database connection in a global variable:
so you could replace all instances of connection with eg. global.dbConnection:
Hi @prowebat
should I use singleton to solve this same problem?

@seejamescode
Copy link
Author

Overall, my issue was reading blog posts about database connections in serverless setups too fast. I was calling mongooseConnect() inside of my api functions instead outside of them. I do not really encounter this issue as much now.

Feel free to keep the issue open if you think it would still be a beneficial feature request for others. Otherwise, I am fine with closing it for user error!

@vixeven
Copy link

vixeven commented Apr 5, 2020

@seejamescode Can you add please a guide or an example with your approach?

@katywings

This comment has been minimized.

@vvo
Copy link
Member

vvo commented Apr 10, 2020

Hey there, using Next.js 9.3, in dev mode, when an API route is rebuilt, and this API route references db.js where you initiate your database connection, then it will reconnect (and previous connections are just kept alive somehow indefinitely).

When using knex/objection.js/node-postgres (trying to put keywords for SEO for people with similar issues) then you'll get the infamous "sorry, too many clients already".

@Timer how do you handle that on your side? Every Next.js project using a database connection should have the issue at some point.

I have not managed to find a way to express "Ignore this file for auto reload" when that file is a server side module (like lib/db.js) used by one of the API pages.

Thanks!

@vvo
Copy link
Member

vvo commented Apr 10, 2020

Update: I was able to ignore a file to be reloaded by Next.js on change by using https://nextjs.org/docs/api-reference/next.config.js/custom-webpack-config and:

    webpackDevMiddleware(config) {
      config.watchOptions.ignored.push(path.join(__dirname, "lib/db.js"));
      return config;
    },

But the issue is still that connection between different routes is not reused, as each route re-runs the lib/db.js file which is confusing. In a regular express.js server every file require is cached. Weird!

@vvo
Copy link
Member

vvo commented Apr 10, 2020

Note: while I was able to reproduce this once, I tried again and was not able to get constant connection leaking so I guess we can just close this. There's definitely a bug somewhere, most probably not in Next.js though. If someone else ends up here let me know!

@vvo
Copy link
Member

vvo commented Apr 10, 2020

Ok I managed to pinpoint exactly the issue I have been facing with knex connection pooling, see here: knex/knex#3788

@vvo
Copy link
Member

vvo commented Apr 10, 2020

@Timer Do you think it would be feasible to have an option to say "Do not clear require cache for this module" so that we can avoid bugs on some libraries in dev mode? Thanks!

@brandonweiss
Copy link

Oh man, this one really got me. I’ve been using Prisma and Next.js and could not for the life of me figure out why periodically my database would error saying there were too many connections. I opened an issue on the Prisma client to figure it out. At some point I realized that the dev server was reloading the file where the Prisma client is instantiated, eventually creating multiple clients and thus too many connections.

I tried @vvo’s recommendation of adding the file to config.watchOptions.ignored but that doesn’t seem to work for me. If I throw a console.log in that file I can see it’s getting reloaded pretty much every time a file changes.

This is one of those edge-casey situations where technically neither the database library nor Next are doing anything wrong, but in combination you get really unexpected results. I suspect the only way to really this would be with an official way to make Next ignore a file in dev mode?

@huv1k
Copy link
Contributor

huv1k commented Apr 23, 2020

You can do workaround like this meanwhile

let prisma: PrismaClient
if (process.env.NODE_ENV !== 'production') {
  if (!global.prisma) {
    global.prisma = new PrismaClient({
      debug: true,
    })
  }
  prisma = global.prisma
} else {
  prisma = new PrismaClient()
}

@brandonweiss
Copy link

@huv1k 🙌🏼🙌🏼🙌🏼 I completely forgot about global—thanks so much! ❤️

@vvo
Copy link
Member

vvo commented Apr 23, 2020

@huv1k you're the real MVP, I solved my issue this way too:

const knex = require("knex");
let pg;

if (process.env.NODE_ENV === "development") {
  global.pg =
    global.pg ||
    knex({
      client: "pg",
      connection: process.env.DATABASE_URL,
    });

  pg = global.pg;
} else {
  pg = knex({
    client: "pg",
    connection: process.env.DATABASE_URL,
  });
}

@icflorescu
Copy link

Had the same issue for the past 10 months while working on a large project with Next.js + Objection.js/Knex + PostgreSQL.
Tried lots and lots of possible workarounds to no avail, until I found this thread and implemented the following hack based on @huv1k's suggestion:

/**
 * @returns {import('knex')}
 */
const getConnection = () => {
  if (PRODUCTION) return knex(DB);
  /**
   * This `global` hack solves a long-standing issue appearing in development more,
   * when successive hot-reloads lead to connections being leaked and eventually produce
   * an unrecoverable error
   */
  /* eslint-disable no-underscore-dangle */
  if (!global.__KNEX_DB_CONNECTION__) global.__KNEX_DB_CONNECTION__ = knex(DB);
  return global.__KNEX_DB_CONNECTION__;
  /* eslint-enable */
};

const db = getConnection();

export default db;

Thanks a lot! 🙏🏻

@mattcarlotta
Copy link
Contributor

mattcarlotta commented Oct 2, 2020

I'm not so sure polluting the global space is the best work-around. This can be confusing as to where it's being defined (globally, but from what file?) and where it should be used (client? server?). An alternative approach I've found has been to utilize next's runtime configs to prevent connection leaks in development. This also has the added benefit of hinting at where the function is being defined (from next/config which to points to the next.config.js file) and where it should be used (on the server).

On further testing, doesn't appear to work with getStaticProps and getStaticPaths. On the same note, the global example mentioned above still creates duplicate multiple connections in development on each page transition and creates getStaticProps usage + dynamic page n connections in production.

Duplicate connections from navigating back and forth between index and the same dynamic page (3 iSSG pages):

@karlhorky
Copy link
Contributor

karlhorky commented Oct 23, 2020

I'm also having a similar issue with Next.js where the database connections of Postgres.js start to accumulate during a development workflow (page reloading, Fast Refresh, etc), leading to the dreaded PostgreSQL error:

Error: remaining connection slots are reserved for non-replication superuser connections

@huv1k is the globalThis option you mentioned above still the recommended workaround? Or is there a new way of getting around this in Fast Refresh? Would be great to have a hook into the refresh lifecycle to be able to terminate any open connections.

let sql;

if (process.env.NODE_ENV === 'production') {
  sql = postgres({ ssl: true });
} else {
  if (!globalThis.__postgresSqlClient) {
    globalThis.__postgresSqlClient = postgres();
  }
  sql = globalThis.__postgresSqlClient;
}

@karlhorky
Copy link
Contributor

@huv1k Actually, would the Next.js team be open to a new page in the docs about "Database Connections and Other Side Effects"?

I would be open to writing a short guide on how to create singletons for this using globalThis, if this is the current official recommendation.

CC @Timer @timneutkens

@noclat
Copy link

noclat commented Apr 11, 2021

A bit late here but you guys all saved my life. I'm using Bull and each API call would create new Redis clients, so I had to restart the server frequently (like every 10 minutes) when developing because of memory leaks.

I package all my instances in a src/server/services/index.js file where I init databases, Bull and other third-party services (SDKs). So I've extracted the global logic into a helper:

const registerService = (name, initFn) => {
  if (process.env.NODE_ENV === 'development') {
    if (!(name in global)) {
      global[name] = initFn();
    }
    return global[name];
  }
  return initFn();
};

So that I can create unique instances of my services by simply calling:

// src/server/services/index.js
export const db = registerService('db', () => knex({
  client: 'pg',
  connection: process.env.DATABASE_URL,
}));

// src/pages/api/...
import { db } from 'server/services';

for example.

I've spent so many hours trying to find a way to fix that. And this works perfectly. Thanks a lot!

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests