Allow TypeormStore to recover from failed request

[Kehrlann]

Hey there 👋
First, thanks for the lib !

We had an issue in prod the other day ; our (Postgres) database threw an exception at some point while fetching the sessions. However it was just a flake, not a complete crash of the connection. When this happens, the TypeormStore emits a disconnect event, and then never reconnects. We did a quickfix but that's nasty (see below).

I don't think the store should disconnect on every time there's an exception. Not sure what the ideal behavior would be:

• Do not disconnect, just log the errors ? Most of the connect-xyz implementations do not emit connect/disconnect events, see Redis: https://github.com/tj/connect-redis/
• Buffer the errors and only disconnect after a certain number of errors ?
• Include a "reconnect" mechanism ?
• Include a "circuit-breaker" type mechanism ?
• Give the users a way to define a failure handler ?

Any thoughts ?

---

For reference, our quickfix is to do something like:

  const store = new TypeormStore({
    cleanupLimit: 2,
    ttl: 86400,
  })
    .connect(sessionRepository)
    .on("disconnect", () => setTimeout(() => store.emit("connect"), 100));

Note: The timeout is to give the disconnect event more time to propagate before we fire a connect event, I think even a 0 timeout would work.

[nykula]

Hello! I like how "Give the users a way to define a failure handler" sounds. What do you think of an option like onError: (s: Store, e: Error) => {...}?
Could you please add a failing test case that illustrates the current issue, or link me a minimal repo with reproduction? (:

[Kehrlann]

I like onError. However, what should the default behavior be ? And should onError come on top the current implementation, or replace it entirely ?

I added a (quick and dirty) failing test case in #20

[nykula]

Hello again, sorry for the delay, have been busy over the week. onError keeps the debug line but is called right after it instead of the disconnect event if present. Please confirm that the commit dd5516e works for your case so I publish the patch to npm, or reopen the issue if it doesn't.

[Kehrlann]

Yep that works great, thanks !