voxpelli / node-connect-pg-simple

A simple, minimal PostgreSQL session store for Express
https://www.npmjs.com/package/connect-pg-simple
MIT License
234 stars 74 forks source link

Using "pruneSessionInterval=false" will unexpectedly disable "createTableIfMissing=true" #279

Closed Reiddd closed 1 year ago

Reiddd commented 1 year ago

Hi :)

I meet a weird issue in my product with connect-pg-simple 7.0.0. Here is my code

import PostgresStoreFactory from 'connect-pg-simple';
import session from 'express-session';

const PostgresStore = PostgresStoreFactory(session);
this.store = new PostgresStore({
  pool: pgPool,
  createTableIfMissing: true,
  errorLog: (...args) => {
    logger.error('Postgres session backend error:', ...args);
  },
  pruneSessionInterval: false,
});

Normally I would expect that the session table can be automatically created since I specified createTableIfMissing=true, but in fact it's not.

Error: relation "session" does not exist

Removing pruneSessionInterval=false fixes it, but I think these two parameters should not have dependency on each other, so this looks like a bug to me.

voxpelli commented 1 year ago

This is odd, as all the integration tests for the auto-creation of tables are run with pruneSessionInterval: false, see https://github.com/voxpelli/node-connect-pg-simple/blob/920583fadbe4e78f3dda4c077837ad0c8ff0ef9a/test/integration/basic.spec.js#L29

Are you sure pruneSessionInterval: false is the cause? I typically go about fixing a bug like this by creating a test case that reproduces the failure and then fixing the code until that test case passes, but in this case there are already test cases that shows that this should be working

Reiddd commented 1 year ago

I'm pretty sure, but your test case looks more persuasive x_x Anyway, in my product we have bypassed this issue, so I'm ok if you want to close this. I will keep an eye on this kind of issue in my product to look for the real root cause if there is one

voxpelli commented 1 year ago

Sorry, didn't see your comment before I added the label 🙈 Closing for now then, open a new one if the issue pops up again