Asynchronous Handling Issue in API Key Hashing Middleware #3338
Conversation
Previously, next() was being called after hashing each API key, which caused multiple calls and issues. The new solution uses a pendingTasks counter to wait until all async operations are completed. This fix prevents Mongoose errors and premature next() calls, ensuring proper API key hashing.
server/models/user.js
Outdated
| if (!user.isModified('apiKeys')) { | ||
| next(); | ||
| return; | ||
| return next(); |
There was a problem hiding this comment.
This change is a lot neater!
There was a problem hiding this comment.
I think overall this looks good so far and is actually pretty close to being good to go!
I think one more thing that needs to happen before we merge it is to address the linting errors. So far it seems to pass the other tests, so I think this should be done once that's addressed. I strongly recommend running 'npm run lint' and 'npm run test' on your pull requests before submitting them just to ensure that they're good to go!
now these may work on npm run lint and able to get merge
|
@raclim i fixed the linting test. now you may merge these from my end. |
There was a problem hiding this comment.
This was the correct one that I meant to approve and merge in 😂 I think there could be some potential further tweaks here in terms of refactoring, but I feel like that could probably be addressed in subsequent PRs and is good for now! Thanks so much again for your work on this!
2 participants
Previously, next() was being called after hashing each API key, which caused multiple calls and issues. The new solution uses a pendingTasks counter to wait until all async operations are completed. This fix prevents Mongoose errors and premature next() calls, ensuring proper API key hashing.
The Code i have been manipulated works on the simple principle of critical section problem simmilar to peterson's solution.
These will fix the issue easily .
Fixes #issue-[3016](url)
Changes:
I have verified that this pull request:
npm run lint)npm run test)developbranch.Fixes #123