createTableIfMissing option not working

[Chaphasilor]

Hey there,

just got around to trying this out, took a bit longer than anticipated ^^

Anyway, I struggled quite a bit at first because I trusted in the createTableIfMissing option to create the table, but it never did that.
This resulted in errors, which weren't really shown (probably because express-session only shows them in debug mode, which is odd). I had to manually add debug logging to access the error.

Maybe you could add some warnings if the table doesn't exists, etc.? (And fix the option, naturally 😉)

[Minigugus]

Hi @Chaphasilor, thanks for trying this out 👍

For the createTableIfMissing I preserved connect-pg-simple's behavior: the table is created lazily, only when the first sql query occurs. Starting the server is not enough to create the session table. Therefore, any table creation error is thrown from the express-session middleware and can be caught by an error middleware and displayed to the user.

However, it is possible there's an issue with the table and schema name not properly escaped. I tested the createTableIfMissing property (see test.js), and it worked without an issue. It only failed with a stacktrace from express when the schema or table name included a space or any not properly escaped character. I just created a branch that should address the sql identifier issue. If you provide your own table or schema name, could you give it a try please? You can test it with npm i Minigugus/connect-postgres-simple#%40fix/create-table.

[Chaphasilor]

Hmm, I'm using the default table name, so that's not it.

My debug logging showed that the error was thrown in the get() method, because it was trying to load the session data from a non-existing table.
Maybe some explicit error handling could help here, but shouldn't the table be created before any db access, including the get()?

[Minigugus]

My debug logging showed that the error was thrown in the get() method, because it was trying to load the session data from a non-existing table.

That's strange because during my tests, table creation errors were thrown and displayed even without debug enabled. Does the test.js script works with your config? (When running, you can trigger a session update with curl 'http://localhost:3000/hello' -H 'Content-Type: application/json' --data-raw '{"some":"data"}')

shouldn't the table be created before any db access

The problem is where and how to report the error if it fails? A database call when creating the store instance would require top-level-awaitandconsole.errorins't really clean. I guess that's what ledconnect-pg-simple` to do it this way, and I'd like to keep the public API close. Anyway, I don't think this is the root cause of your problem.

[Chaphasilor]

Okay, I think I figured it out.
connect-postgres-simple only creates the table if it doesn't exist on first db access. So if I do

1. drop table
2. start server
3. make session request

everything works as expected.

Edit:
Seems like it only works with your test.js example, not with my actual server. Could it be that not all methods check if the table exists on first access? I believe I have some middleware that first checks for an existing session before creating a new one...

However, if I delete the table while the server is running, it doesn't check if the table exists anymore, so postgres throws the error.

Checking if the table exists is probably a bit too much overhead do to with each db access, so I think the behavior is fine.
I have no idea why it didn't work for me at first, I'm pretty sure that I restarted the server multiple times ¯\_(ツ)_/¯
Maybe I had the postgres:// URL wrong at first or something :)

Maybe you could try to confirm this behavior and include a notice in the readme before closing the issue 😉

[Minigugus]

I believe I have some middleware that first checks for an existing session before creating a new one...
[...]
and include a notice in the readme

You're right, sorry, I forgot express-session only allocate a session when the session object is updated, i.e. no sql request is made and the table is never created. In my test.js it worked because was aware of that behavior, I updated the session object. I agree it's confusing, but it's the same algorithm as connect-pg-simple. The README is also a copy from connect-pg-simple, and I'm surprised the readme doesn't already contains information about that behavior. Does your code works differently with connect-pg-simple? Maybe I'm missing something, or if not, connect-pg-simple might be concerned too.

Would you like a dedicated method for creating the table programmatically? await connectPostgresSimple.createTable() for instance? Recent Node versions support top-level-await so it could be acceptable I guess (while not breaking compatibility with connect-pg-simple)?

[Chaphasilor]

Does your code works differently with connect-pg-simple?

never used that, I'm switching over from MySQL :)
The thing is, I'm actually getting errors from passport.js before the error from postgres, but only when using connect-postgres-simple's session store and the session table doesn't exist yet. Maybe the API isn't compatible with passport...

I'm still trying to figure out where exactly the error is originating, if I find out anything new I'll let you know.

Would you like a dedicated method for creating the table programmatically?

Don't think that's necessary. If anything it should be created automatically :)

[Minigugus]

Maybe the API isn't compatible with passport...

I had a look at the passport.js documentation and it seems it don't use the store directly but the express-session middleware instead, so I don't see how the store could be an issue...

I'm still trying to figure out where exactly the error is originating, if I find out anything new I'll let you know.

Maybe I could help you more with some logs/code sample, if you can share them?

If anything it should be created automatically

The problem is where to report any creation error. I could start the table creation asynchronously, but it may cause an unhandled promise rejection and crash the process on recent Node versions 😕 That's why I think a dedicated method is the way to go, as it let the user decide whether or not and how to deal with rejections. connect-pg-simple also do that with the prune method to let the user delete expired sessions

[Chaphasilor]

But would the method replace the createTableIfMissing option or supplement it?

I can send you some more logs tomorrow :)