-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
"Record to delete does not exist" error #91
Comments
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! |
Same error. |
Thanks for checking and updating, @develoQ |
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! |
Hi @develoQ,
Can you elaborate? Do you mean that if you change certain Also (@develoQ and @shayneczyzewski ) it looks like all calls to |
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. |
@shayneczyzewski - got it. Hopefully the update improves on this situation; if not, please let us know. |
This error seems to occur in 'catch'. Therefore, this error must be re-catch or the error will be thrown
|
Hi, I faced similar issue, is there any other alternatives before this issue fixed? |
Hi @fakhruz3105 - not that I know of; feel free to post a PR, if you see a fix though. |
@develoQ wrote:
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. |
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 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:
Ref: https://github.com/jaredhanson/passport/blob/master/CHANGELOG.md |
Hi @ecker00 - thanks for this additional context.
Just to be clear - is this causing a functional problem, a crash, or simply an error message? The library itself wraps all One idea: Maybe under the hood, the passport library relies on If you look in passport's code, do you see anything like this? (Currently, this library supplies 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! |
Correct, it's just displaying the error message 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
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. |
Ok - lines deleted in PR #98. |
Thank you for such a swift update! 👍 |
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 I'm not sure which is the correct behavior:
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 |
@kleydon I'm still experiencing this issue after upgrading to This occurs when logging in when there is no existing session.
|
@kleydon This issue does not exist when I use the SQLiteStore. Therefore, I suspect that the problem could be fixed within this repository. |
@mfidor This happens for me even with |
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 |
Here is my temporary hack until this issue is resolved:
|
I created a pr with this issue fixed #102 (comment) |
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:
I get the following error:
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):
I got the following error:
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 aregenerate
.Here is the full session store config in which the above lives:
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!
The text was updated successfully, but these errors were encountered: