jcs224 / hono_sessions

Cookie-based sessions for Hono applications
https://jsr.io/@jcs224/hono-sessions
MIT License
53 stars 6 forks source link

Optimize session deletion, recreation, and persistence #10

Closed callmetwan closed 2 months ago

callmetwan commented 3 months ago

This work optimizes control flow by only rotating the session key or persisting data if the session is not deleted. The flow before this work would recreate the session key/persist session data every time, even if it would end up being deleted. This is important if storing the session in a database, as that means there is at least one additional round trip. For larger systems this could mean cutting the number of database interactions in half.

Tangential, the bun dev script has been split into two to reflect the two types of servers you can run.

Before merging I would love it if someone else could test this as well. Not being overly familiar with the library and there being no unit tests makes me nervous I missed something 😅

callmetwan commented 3 months ago

A few other notes:

jcs224 commented 3 months ago

Looks like something broke the cookie driver.

image

callmetwan commented 3 months ago

I'll take a look later this week

callmetwan commented 3 months ago

Hmmm, running it locally all the tests pass. Here is the node cookie test for example

image

It seems to be failing on the incorrect password step but I verified in Chrome and FF (on a Mac) that they work as expected. Is there a way to re-run the tests in GH actions? I looked but didn't see any options.

jcs224 commented 3 months ago

Weird. Did you change the STORE environment variable to cookie before running the test?

I'll run the action again.

jcs224 commented 3 months ago

Still failing, I'll check this out in the next couple days.

callmetwan commented 3 months ago

Ah, I didn’t realize I needed to pass an encryption key. I’ll remind myself to add this to the testing section of the README

callmetwan commented 3 months ago

Okay, think I fixed it! Had a ! in the wrong place

jcs224 commented 3 months ago

Nice! I might play with it a little before I merge it in, but this looks good, thanks!

callmetwan commented 3 months ago

Please do, I want to be 100% sure I didn't mess anything up unintentionally! And glad to do it :)