kleydon / prisma-session-store

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

"Record to delete does not exist" error #91

Closed shayneczyzewski closed 1 year ago

shayneczyzewski commented 1 year ago

Hi there, we are considering using this library to help us with our Express + Prisma DB-backed sessions at Wasp. In trying it out locally and on Heroku, I found I get similar failures related to deletion that I am unable to diagnose.

For example, locally, when setting:

  resave: false,
  saveUninitialized: false,

I get the following error:

Server: Error: 
Server (stderr): Invalid `this.prisma[this.sessionModelName].delete()` invocation in
Server (stderr): /Users/shayne/dev/wasp/waspc/examples/todoApp/.wasp/out/server/node_modules/@quixo3/prisma-session-store/dist/lib/prisma-session-store.js:266:91
Server (stderr): 
Server (stderr):   263 case 3:
Server (stderr):   264     _a.sent();
Server (stderr):   265     return [3 /*break*/, 6];
Server (stderr): → 266 case 4: return [4 /*yield*/, this.prisma[this.sessionModelName].delete(
Server (stderr):   An operation failed because it depends on one or more records that were required but not found. Record to delete does not exist.
Server (stderr):     at cb (/Users/shayne/dev/wasp/waspc/examples/todoApp/.wasp/out/server/node_modules/.prisma/client/runtime/index.js:38703:17)
Server (stderr):     at async PrismaClient._request (/Users/shayne/dev/wasp/waspc/examples/todoApp/.wasp/out/server/node_modules/.prisma/client/runtime/index.js:40853:18)
Server (stderr): set(): Error: 
Server (stderr): Invalid `this.prisma[this.sessionModelName].create()` invocation in
Server (stderr): /Users/shayne/dev/wasp/waspc/examples/todoApp/.wasp/out/server/node_modules/@quixo3/prisma-session-store/dist/lib/prisma-session-store.js:493:85
Server (stderr): 
Server (stderr):   490 case 4:
Server (stderr):   491     _a.sent();
Server (stderr):   492     return [3 /*break*/, 7];
Server (stderr): → 493 case 5: return [4 /*yield*/, this.prisma[this.sessionModelName].create(
Server (stderr):   Unique constraint failed on the fields: (`id`)
Server (stderr): POST /auth/login 200 142.790 ms - 1002
Server: Error: 
Server (stderr): Invalid `this.prisma[this.sessionModelName].create()` invocation in
Server (stderr): /Users/shayne/dev/wasp/waspc/examples/todoApp/.wasp/out/server/node_modules/@quixo3/prisma-session-store/dist/lib/prisma-session-store.js:493:85
Server (stderr): 
Server (stderr):   490 case 4:
Server (stderr):   491     _a.sent();
Server (stderr):   492     return [3 /*break*/, 7];
Server (stderr): → 493 case 5: return [4 /*yield*/, this.prisma[this.sessionModelName].create(
Server (stderr):   Unique constraint failed on the fields: (`id`)
Server (stderr):     at cb (/Users/shayne/dev/wasp/waspc/examples/todoApp/.wasp/out/server/node_modules/.prisma/client/runtime/index.js:38703:17)
Server (stderr):     at async PrismaClient._request (/Users/shayne/dev/wasp/waspc/examples/todoApp/.wasp/out/server/node_modules/.prisma/client/runtime/index.js:40853:18)
Server (stderr): 

I did see the session was actually persisted, however.

And on Heroku, with the following settings (interestingly these did work locally, but other permutations caused issues locally):

  resave: false,
  saveUninitialized: true,

I got the following error:

2022-06-14T17:37:12.221722+00:00 app[web.1]: Error: An operation failed because it depends on one or more records that were required but not found. Record to delete does not exist.
2022-06-14T17:37:12.221732+00:00 app[web.1]:     at cb (/app/server/node_modules/.prisma/client/runtime/index.js:38703:17)
2022-06-14T17:37:12.221733+00:00 app[web.1]:     at async PrismaClient._request (/app/server/node_modules/.prisma/client/runtime/index.js:40853:18)
2022-06-14T17:37:12.229882+00:00 app[web.1]: set(): Error: Unique constraint failed on the fields: (`id`)
2022-06-14T17:37:12.230167+00:00 app[web.1]: POST /auth/login 200 284.895 ms - 148
2022-06-14T17:37:12.230431+00:00 app[web.1]: Error: Unique constraint failed on the fields: (`id`)
2022-06-14T17:37:12.230432+00:00 app[web.1]:     at cb (/app/server/node_modules/.prisma/client/runtime/index.js:38703:17)
2022-06-14T17:37:12.230433+00:00 app[web.1]:     at async PrismaClient._request (/app/server/node_modules/.prisma/client/runtime/index.js:40853:18)

We are using Prisma v3.9.1 and a callback style similar to this example for our login sessions: https://github.com/expressjs/session#user-login where we save inside a regenerate.

Here is the full session store config in which the above lives:

const sessionConfig = {
  name: config.session.name,
  secret: config.session.secret,
  resave: false,                        // ☝️
  saveUninitialized: false,      // ☝️
  cookie: {
    httpOnly: true,
    maxAge: config.session.cookie.maxAge,
  },
  store: new PrismaSessionStore(prisma, {
    checkPeriod: 2 * 60 * 1000,  //ms
    dbRecordIdIsSessionId: true,
    dbRecordIdFunction: undefined
  })
}

Are the errors I am getting due to this style of saving the session, or perhaps from the config settings? Anything else I can do to help debug this, as I'm not entirely sure what the library is attempting to do there with the deletions? Thanks!

ivenuss commented 1 year ago

Hello, I'm having similar issue using passport in #81 my issue.

kleydon commented 1 year ago

Hi @shayneczyzewski - thanks for the bug report.

I'm using the same parameter values - resave: false, saveUninitialized: false - in one project; will keep a look out to see if I encounter this error.

Note: A recent fix from PR #93 may improve matters; not yet sure.

shayneczyzewski commented 1 year ago

Hi @shayneczyzewski - thanks for the bug report.

I'm using the same parameter values - resave: false, saveUninitialized: false - in one project; will keep a look out to see if I encounter this error.

Note: A recent fix from PR #93 may improve matters; not yet sure.

Thanks @kleydon, I'll give it a go when I get some time this week and let you know. I appreciate your prompt attention!

tequdev commented 1 year ago

Same error.

93 does not resolve this.

shayneczyzewski commented 1 year ago

Same error. #93 does not resolve this.

Thanks for checking and updating, @develoQ

tequdev commented 1 year ago

@kleydon

If I use deleteMany, it works fine.

Can you keep changing to deleteMany until you get to the root of the problem?

It is better to have working code up-to-date than non-working code up-to-date in the repository!

kleydon commented 1 year ago

Hi @develoQ,

If I use deleteMany, it works fine. Can you keep changing to deleteMany until you get to the root of the problem?

Can you elaborate?

Do you mean that if you change certain delete() calls in the prisma-session-store library to deleteMany(), you don't encounter the error message? Or do you mean that if you use deleteMany() instead of delete() in your own back-end code you don't encounter the error message?

Also (@develoQ and @shayneczyzewski ) it looks like all calls to delete() and deleteMany() made from within the library itself occur within try/catch blocks; directly, or indirectly (in the case of the prune() function) - suggesting that although the error message may appear (in circumstances we are still trying to narrow down), the errors are not causing crash behavior... Are you just seeing undesirable/not-yet-explained error messages - or are you seeing crashing behavior?

shayneczyzewski commented 1 year ago

Hi @kleydon, it was preventing my app from proceeding normally for me when I opened the issue, not just log messages. I have not yet been able to test the update.

kleydon commented 1 year ago

@shayneczyzewski - got it. Hopefully the update improves on this situation; if not, please let us know.

tequdev commented 1 year ago

This error seems to occur in 'catch'.

Therefore, this error must be re-catch or the error will be thrown

https://github.com/kleydon/prisma-session-store/blob/6dac2a222195402d5bf448c43edb22b00fdf79ab/src/lib/prisma-session-store.ts#L223

fakhruz3105 commented 1 year ago

Hi, I faced similar issue, is there any other alternatives before this issue fixed?

kleydon commented 1 year ago

Hi @fakhruz3105 - not that I know of; feel free to post a PR, if you see a fix though.

kleydon commented 1 year ago

@develoQ wrote:

This error seems to occur in 'catch'. Therefore, this error must be re-catch or the error will be thrown

Interesting. Can you post a trace that shows this error happening - or explain what might be going wrong in the catch block? I don't yet understand what the error could be.

ecker00 commented 1 year ago

I recently started seeing this error as well after updating a few NPM packages. After a bit of digging I see that this started happening when updating passport from version 0.5.3 to 0.6.0.

When I run a login operation, it also seems to run a session delete operation too, which fails when it can't find a session to delete.

Edit: Reading PassportJS change log, it says:

Ref: https://github.com/jaredhanson/passport/blob/master/CHANGELOG.md

kleydon commented 1 year ago

Hi @ecker00 - thanks for this additional context.

When I run a login operation, it also seems to run a session delete operation too, which fails when it can't find a session to delete.

Just to be clear - is this causing a functional problem, a crash, or simply an error message?

The library itself wraps all delete() / deleteMany() calls in try/catch logic, so attempting to delete a session which doesn't exist shouldn't itself cause a problem.

One idea: Maybe under the hood, the passport library relies on session.destroy(sid, callback)'s optional callback, and assumes that this callback should execute normally in the case where the session to destroy is not found?

If you look in passport's code, do you see anything like this?

(Currently, this library supplies callback with an error argument when the session to destroy is not found; see prisma-session-store.ts, line 223: if (callback) defer(callback, e);).

The express-session library doesn't specify (as far as I can tell) whether a failure to destroy a session because a session doesn't exist should result in the callback being processed with an error argument or not; we'd have to think through what the implications of changing this would be.

I think a number of user are using passport. If you come up with any solution here, I'm sure people love to hear it!

ecker00 commented 1 year ago

Correct, it's just displaying the error message logger.warn(), as it's wrapped in a try/catch. But looks a bit serious when you see this flooding the server logs.

Digging a bit through passport's latest commits between 0.5.3 and 0.6.0, I believe maybe this is the relevant change, where login it will now run req.session.regenerate(…), which is an express-session function (see here and here) that in turn calls destroy() and then generate().

… and assumes that this callback should execute normally in the case where the session to destroy is not found?

By the looks of it, the callback will always be called for both success and exceptions, and pass any error along, if any.

I think the solution would be to simply delete line 220-202, and not display the warning. There is a callback here which will pass on this error message for cases you want to handle this exception.

kleydon commented 1 year ago

I think the solution would be to simply delete line 220-202, and not display the warning. There is a callback here which will pass on this error message for cases you want to handle this exception.

Ok - lines deleted in PR #98.

ecker00 commented 1 year ago

Thank you for such a swift update! 👍

mfidor commented 1 year ago

Hi! I'm experiencing a similar issue. It happens when trying to destroy or regenerate a session already removed from the database.

For me, it happens when I'm signing out. The request is done on the front-end in a useEffect hook, but because of the React's Strict Mode, the hook is called twice, effectively making two requests to the API. That triggers the error, because the session has already been destroyed on the second request.

I'm not sure which is the correct behavior:

  1. should the error be returned as it is now, because the session does not exist at this point and we should know about it,
  2. or should it just attempt to delete the session without checking if it exists in the store. The session is gone after all, which is our expected outcome.

In the second case, the store could return error only when there is something wrong with the execution of the query, like when the database is unreachable.

Anyway, I found a workaround for myself, and maybe someone will find it helpful.

await new Promise((resolve, reject) => {
  req.session.destroy((err) => {
    // ignore an error with code "P2025", which is thrown when the session is not present in the database.
    if (err && err.code !== "P2025") {
      console.error(err);
      reject("Could not sign out.");
    } else {
      res.clearCookie("id");
      resolve();
    }
  });
});

EDIT: A similar problem occurs when I regenerate a session that was not yet saved (when using the saveUninitialized: false setting). It then tries to delete a session that was never saved to the store.

YaakovR commented 1 year ago

@kleydon I'm still experiencing this issue after upgrading to passport 0.6.0.

This occurs when logging in when there is no existing session.

Error: 
Invalid `this.prisma[this.sessionModelName].delete()` invocation in
C:\Projects\finance\todos-express-password\node_modules\@quixo3\prisma-session-store\dist\lib\prisma-session-store.js:266:91

  263 case 3:
  264     _a.sent();
  265     return [3 /*break*/, 6];
→ 266 case 4: 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 RequestHandler.handleRequestError (C:\Projects\finance\todos-express-password\node_modules\@prisma\client\runtime\index.js:28658:13)
    at RequestHandler.request (C:\Projects\finance\todos-express-password\node_modules\@prisma\client\runtime\index.js:28640:12)
    at async consumer (C:\Projects\finance\todos-express-password\node_modules\@prisma\client\runtime\index.js:29618:18)
    at async PrismaClient._request (C:\Projects\finance\todos-express-password\node_modules\@prisma\client\runtime\index.js:29639:16)
YaakovR commented 1 year ago

@kleydon This issue does not exist when I use the SQLiteStore. Therefore, I suspect that the problem could be fixed within this repository.

YaakovR commented 1 year ago

EDIT: A similar problem occurs when I regenerate a session that was not yet saved (when using the saveUninitialized: false setting). It then tries to delete a session that was never saved to the store.

@mfidor This happens for me even with saveUninitialized: true. To replicate, try manually deleting session just before the login occurs.

mfidor commented 1 year ago

Yes, it fails in every situation where a session should be deleted, and it's not in the database.

@kleydon, I think the call to delete should be replaced with deleteMany as it was already suggested. ThedeleteMany method will not throw an error when nothing matches the criteria. Wether it gets deleted or already does not exist, the session is not in the database, which is the expected outcome.

YaakovR commented 1 year ago

Here is my temporary hack until this issue is resolved:

router.post('/login', passport.authenticate('local'), (req, res, next) => {
    res.sendStatus(200);
  }, (err, req, res, next) => {

    if (err.code === 'P2025' && err.meta?.cause === 'Record to delete does not exist.') {
      return res.redirect(307, req.protocol + '://' + req.get('host') + req.originalUrl);
    }

    next(err);
  },
);
geefuoco commented 1 year ago

I created a pr with this issue fixed https://github.com/kleydon/prisma-session-store/pull/102#issue-1345392142

kleydon commented 1 year ago

This issue should now be fixed in master, based on PR #102 - thanks to @geefuoco.

(Please post if it resurfaces.)