-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
feat: Log more debug info when failing to set duplicate value for field with unique values #9919
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
feat: Log more debug info when failing to set duplicate value for field with unique values #9919
Conversation
|
I will reformat the title to use the proper commit message syntax. |
|
🚀 Thanks for opening this pull request! |
📝 WalkthroughWalkthroughAdds server-side logging of MongoDB E11000 duplicate-key error messages in two adapter paths and extends a test to spy on logger output while keeping the client-facing duplicate-value error unchanged. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Adapters/Storage/Mongo/MongoStorageAdapter.js (1)
744-753: Apply formatter consistently inensureUniqueness.The DUPLICATE_VALUE error thrown in this method doesn't use
mongoUniqueIndexErrorFormatter, creating inconsistent error messages across the codebase. Users would see different error formats depending on which code path triggers the duplicate key violation.Apply this diff for consistency:
.catch(error => { if (error.code === 11000) { throw new Parse.Error( Parse.Error.DUPLICATE_VALUE, - 'Tried to ensure field uniqueness for a class that already has duplicates.' + `Tried to ensure field uniqueness for a class that already has duplicates.${mongoUniqueIndexErrorFormatter(error.message)}` ); } throw error; })
🧹 Nitpick comments (1)
spec/schemas.spec.js (1)
3811-3841: Consider more flexible error message assertion.The test uses an exact string match with hardcoded database and collection names. This makes the test brittle—it will break if test configuration changes (database name, collection prefix) even though the functionality works correctly.
Consider using a pattern-based assertion instead:
- expect(error.message).toEqual('A duplicate value for a field with unique values was provided. Duplicate index: code_1 on collection test_UniqueIndexClass in db parseServerMongoAdapterTestDatabase') + expect(error.message).toMatch(/A duplicate value for a field with unique values was provided\. Duplicate index: code_1 on collection \w+UniqueIndexClass in db \w+/);Alternatively, verify the presence of key components:
- expect(error.message).toEqual('A duplicate value for a field with unique values was provided. Duplicate index: code_1 on collection test_UniqueIndexClass in db parseServerMongoAdapterTestDatabase') + expect(error.message).toContain('A duplicate value for a field with unique values was provided'); + expect(error.message).toContain('Duplicate index: code_1'); + expect(error.message).toContain('UniqueIndexClass');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
spec/schemas.spec.js(1 hunks)src/Adapters/Storage/Mongo/MongoStorageAdapter.js(3 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.917Z
Learning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: Tests in the parse-server repository should use promise-based approaches rather than callback patterns with `done()`. Use a pattern where a Promise is created that resolves when the event occurs, then await that promise.
Applied to files:
spec/schemas.spec.js
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`. The preferred pattern is to create a Promise that resolves when an expected event occurs, then await that Promise.
Applied to files:
spec/schemas.spec.js
📚 Learning: 2025-05-04T20:41:05.147Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1312-1338
Timestamp: 2025-05-04T20:41:05.147Z
Learning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`.
Applied to files:
spec/schemas.spec.js
📚 Learning: 2025-10-16T19:27:05.311Z
Learnt from: Moumouls
Repo: parse-community/parse-server PR: 9883
File: spec/CloudCodeLogger.spec.js:410-412
Timestamp: 2025-10-16T19:27:05.311Z
Learning: In spec/CloudCodeLogger.spec.js, the test "should log cloud function triggers using the silent log level" (around lines 383-420) is known to be flaky and requires the extra `await new Promise(resolve => setTimeout(resolve, 100))` timeout after awaiting `afterSavePromise` for reliability, even though it may appear redundant.
Applied to files:
spec/schemas.spec.js
🔇 Additional comments (2)
src/Adapters/Storage/Mongo/MongoStorageAdapter.js (2)
511-529: LGTM! Error message augmentation improves debuggability.The duplicate value error now includes parsed collection and index information, which directly addresses the PR objective of making duplicate key errors easier to debug.
597-607: LGTM! Consistent error message enhancement.The formatter is correctly applied here, maintaining consistency with the
createObjectmethod.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## alpha #9919 +/- ##
=======================================
Coverage 92.53% 92.53%
=======================================
Files 190 190
Lines 15469 15471 +2
Branches 176 176
=======================================
+ Hits 14314 14316 +2
Misses 1143 1143
Partials 12 12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Rahul Lanjewar <[email protected]>
|
Fixed: Security concern - removed schema information from client error responses Changes:
This approach is more robust (no regex parsing to break), secure (no schema info leakage), and provides complete debugging information server-side. |
# [9.1.0-alpha.4](9.1.0-alpha.3...9.1.0-alpha.4) (2025-12-14) ### Features * Log more debug info when failing to set duplicate value for field with unique values ([#9919](#9919)) ([a23b192](a23b192))
|
🎉 This change has been released in version 9.1.0-alpha.4 |
# [9.1.0](9.0.0...9.1.0) (2025-12-14) ### Bug Fixes * Cross-Site Scripting (XSS) via HTML pages for password reset and email verification [GHSA-jhgf-2h8h-ggxv](https://github.com/parse-community/parse-server/security/advisories/GHSA-jhgf-2h8h-ggxv) ([#9985](#9985)) ([3074eb7](3074eb7)) ### Features * Add option `logLevels.signupUsernameTaken` to change log level of username already exists sign-up rejection ([#9962](#9962)) ([f18f307](f18f307)) * Add support for custom HTTP status code and headers to Cloud Function response with Express-style syntax ([#9980](#9980)) ([8eeab8d](8eeab8d)) * Log more debug info when failing to set duplicate value for field with unique values ([#9919](#9919)) ([a23b192](a23b192))
|
🎉 This change has been released in version 9.1.0 |
Pull Request
Issue
Closes: #9891
Approach
Tasks
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.