Skip to content

Conversation

@RahulLanjewar93
Copy link
Contributor

@RahulLanjewar93 RahulLanjewar93 commented Nov 8, 2025

Pull Request

Issue

Closes: #9891

Approach

Tasks

  • Add tests
  • Add changes to documentation (guides, repository pages, code comments)
  • Add security check
  • Add new Parse Error codes to Parse JS SDK

Summary by CodeRabbit

  • Bug Fixes

    • Duplicate-value errors now return a consistent, generic message to clients while full database conflict details are recorded internally in server logs for debugging.
  • Tests

    • Tests updated to assert the client-facing generic duplicate error and to verify that the server logs capture full duplicate-key conflict details for troubleshooting.

✏️ Tip: You can customize this high-level summary in your review settings.

@parse-github-assistant
Copy link

I will reformat the title to use the proper commit message syntax.

@parse-github-assistant parse-github-assistant bot changed the title feat: add information about, db collection and index name on duplicate value error feat: Add information about, db collection and index name on duplicate value error Nov 8, 2025
@parse-github-assistant
Copy link

parse-github-assistant bot commented Nov 8, 2025

🚀 Thanks for opening this pull request!

@coderabbitai
Copy link

coderabbitai bot commented Nov 8, 2025

📝 Walkthrough

Walkthrough

Adds 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

Cohort / File(s) Summary
Mongo storage adapter (logging)
src/Adapters/Storage/Mongo/MongoStorageAdapter.js
Logs duplicate-key errors: when Mongo returns code 11000 in createObject and findOneAndUpdate, the adapter now calls logger.error("Duplicate key error:", err.message) before returning/throwing the existing Parse duplicate-value error. No change to emitted Parse.Error text or control flow.
Test update (logger spy + assertions)
spec/schemas.spec.js
Adds a spy/reset on logger.error and extends assertions to ensure the client-facing error remains the generic duplicate-value message while server logs contain the full MongoDB duplicate-key details (E11000, index name, conflicting value).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Focus areas:
    • Verify correct logger import/usage and proper spy/reset in tests.
    • Ensure tests are robust across environments and restore logger state.
    • Review logging content for potential sensitive data exposure in persisted logs.

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description check ✅ Passed The description includes the required template structure with the issue link (#9891) filled in and test completion marked, though the Approach section lacks implementation details.
Linked Issues check ✅ Passed The PR successfully addresses issue #9891 by adding server-side logging of the full MongoDB error context for duplicate key errors [#9891], providing better debugging information.
Out of Scope Changes check ✅ Passed All changes are directly related to the objectives: adding logging for duplicate key errors in the MongoDB adapter and corresponding test updates, with no unrelated modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title accurately describes the main change: adding debug logging for duplicate key errors. It directly reflects the core modification in both error-handling code paths.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@parseplatformorg
Copy link
Contributor

parseplatformorg commented Nov 8, 2025

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

Copy link

@coderabbitai coderabbitai bot left a 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 in ensureUniqueness.

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

📥 Commits

Reviewing files that changed from the base of the PR and between a85ba19 and 17c8909.

📒 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 createObject method.

@codecov
Copy link

codecov bot commented Nov 8, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.53%. Comparing base (3074eb7) to head (98050b4).
⚠️ Report is 3 commits behind head on alpha.

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

coderabbitai[bot]
coderabbitai bot previously approved these changes Nov 8, 2025
@mtrezza
Copy link
Member

mtrezza commented Dec 14, 2025

Fixed: Security concern - removed schema information from client error responses

Changes:

  • Removed mongoUniqueIndexErrorFormatter() function (fragile regex parsing)
  • Server logs: Now log the complete MongoDB error message via logger.error('Duplicate key error:', error.message)
  • Client response: Returns only generic message: "A duplicate value for a field with unique values was provided" (no database/collection/index names exposed)
  • Test updated: Verifies generic client message + checks server logs contain full MongoDB error details

This approach is more robust (no regex parsing to break), secure (no schema info leakage), and provides complete debugging information server-side.

coderabbitai[bot]
coderabbitai bot previously approved these changes Dec 14, 2025
@mtrezza mtrezza changed the title feat: Add information about, db collection and index name on duplicate value error feat: Log more debug info when providing duplicate value for field with unique values Dec 14, 2025
@mtrezza mtrezza changed the title feat: Log more debug info when providing duplicate value for field with unique values feat: Log more debug info when trying to set duplicate value for field with unique values Dec 14, 2025
@mtrezza mtrezza changed the title feat: Log more debug info when trying to set duplicate value for field with unique values feat: Log more debug info when failing to set duplicate value for field with unique values Dec 14, 2025
@mtrezza mtrezza merged commit a23b192 into parse-community:alpha Dec 14, 2025
21 of 23 checks passed
parseplatformorg pushed a commit that referenced this pull request Dec 14, 2025
# [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))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 9.1.0-alpha.4

@parseplatformorg parseplatformorg added the state:released-alpha Released as alpha version label Dec 14, 2025
parseplatformorg pushed a commit that referenced this pull request Dec 14, 2025
# [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))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 9.1.0

@parseplatformorg parseplatformorg added the state:released Released as stable version label Dec 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state:released Released as stable version state:released-alpha Released as alpha version

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Collection and index name not visible when duplicate value for a unique field is encountered

3 participants