kleydon / prisma-session-store

Express session store for Prisma
MIT License
116 stars 18 forks source link

App crashing with PrismaClientKnownRequestError when failing to delete session #76

Closed jeremygottfried closed 2 years ago

jeremygottfried commented 2 years ago

I suddenly started getting the following error in my app, and it is causing my entire app to crash. I also had a memory exceeded error around the same time, so not sure if this is related, but either way it should fail more gracefully instead of crashing the entire app.

Invalid `this.prisma[this.sessionModelName].delete()` invocation in
/Users/jeremygottfried/Development/sweepstakes-app/sweepstakes-next-app/node_modules/@quixo3/prisma-session-store/dist/lib/prisma-session-store.js:425:87

  422 this.logger.log("session:" + session.sid + " expires in " + remainingSec + "sec");
  423 if (!(now.valueOf() >= session.expiresAt.valueOf())) return [3 /*break*/, 5];
  424 this.logger.log("Deleting session with sid: " + session.sid);
→ 425 return [4 /*yield*/, this.prisma[this.sessionModelName].delete(
  An operation failed because it depends on one or more records that were required but not found. Record to delete does not exist.
    at Object.request (/Users/jeremygottfried/Development/sweepstakes-app/sweepstakes-next-app/node_modules/@prisma/client/runtime/index.js:38864:15)
(node:81352) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 419)
(node:81352) UnhandledPromiseRejectionWarning: Error:
Invalid `this.prisma[this.sessionModelName].delete()` invocation in
/Users/jeremygottfried/Development/sweepstakes-app/sweepstakes-next-app/node_modules/@quixo3/prisma-session-store/dist/lib/prisma-session-store.js:425:87
AlexSPx commented 2 years ago

I'm getting the same problem here

Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client
    at ServerResponse.setHeader (_http_outgoing.js:558:11)
    at ServerResponse.header (D:\Archive\Coding Projects\courselab\server\node_modules\express\lib\response.js:771:10)
    at ServerResponse.append (D:\Archive\Coding Projects\courselab\server\node_modules\express\lib\response.js:732:15)
    at ServerResponse.res.cookie (D:\Archive\Coding Projects\courselab\server\node_modules\express\lib\response.js:857:8)
    at ServerResponse.clearCookie (D:\Archive\Coding Projects\courselab\server\node_modules\express\lib\response.js:804:15)
    at D:\Archive\Coding Projects\courselab\server\dist\api\user.js:182:24
    at Immediate._onImmediate (D:\Archive\Coding Projects\courselab\server\node_modules\@quixo3\prisma-session-store\dist\lib\utils\defer.js:16:18)
    at processImmediate (internal/timers.js:461:21) {
  code: 'ERR_HTTP_HEADERS_SENT'
}
[nodemon] app crashed - waiting for file changes before starting...
jeremygottfried commented 2 years ago

@AlexSPx I'm pretty sure your bug is different. That would happen if the library tries to set headers after the response is already sent.

AlexSPx commented 2 years ago

@jeremygottfried Oh the error I sent is different, but I have gotten yours as well. But I fixed it!! I switched to using connect-redis, the session store for redis. I use it for caching so I decided to just switch to it.

jeremygottfried commented 2 years ago

@kleydon I think this is a race condition caused by having multiple declarations of express-session. Since every declaration starts a new setInterval in this library, I probably have a bunch of intervals running in the background, one for each endpoint?

The reason is my app uses next-connect within a nextjs app, so I am declaring express-session at the handler level. Declaring globally will probably fix this, but I think it would be beneficial to at least catch errors thrown within the setInterval. I'm guessing those weren't being caught in try...catch because they're outside the async system.

kleydon commented 2 years ago

Hey @jeremygottfried - appreciate your diagnosis. It may be a few days before I can carve out the time/head-space to look at this carefully, but - glad that there is now a strong working theory for what is going on.

jeremygottfried commented 2 years ago

@kleydon thanks! I moved my multiple declarations to one express server at top level, and it seems like it's fixed for me. I still think it would be helpful to catch errors thrown inside setInterval, and maybe offer an onError option to handle those errors.

kleydon commented 2 years ago

Hey @jeremygottfried - I finally got around to:

1) Wrapping setInterval()'s prune() invocation in a try/catch block - so that if multiple PrismaSessionStore intervals are running simultaneously, they (hopefully!) won't stomp all over each other...

2) adding an optional onIntevalError callback for setInteval() - so that errors that occur when setInterval()'s prune() statement is called can continue to be surfaced if necessary.

The changes are in a new release on npm: 3.1.5.

Going to close this for now (but if I'm missing something - let me know.)

BTW Thanks for the insight, re: Multiple stores resulting in multiple interval timers running simultaneously - a good thing to be aware of!

jeremygottfried commented 2 years ago

@kleydon Thanks so much!

kleydon commented 2 years ago

👍