fix: MongoDB timeout errors unhandled and potentially revealing internal data #10020
Conversation
|
🚀 Thanks for opening this pull request! |
✅ 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. |
📝 WalkthroughWalkthroughClassifies certain MongoDB errors as transient in MongoStorageAdapter and maps them to a generic Parse.Error("Database error"). Changes middleware error handling in handleParseSession to log the original error and call next() with a sanitized Parse.Error. Adds/updates tests asserting these behaviors. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Middleware
participant StorageAdapter
participant MongoDB
Client->>Middleware: request
Middleware->>StorageAdapter: perform DB operation
StorageAdapter->>MongoDB: execute query/command
rect rgba(255, 100, 100, 0.5)
MongoDB-->>StorageAdapter: transient error (e.g., MongoServerSelectionError)
end
StorageAdapter->>StorageAdapter: isTransientError(error)?
alt transient
StorageAdapter->>StorageAdapter: create Parse.Error(INTERNAL_SERVER_ERROR,"Database error")
StorageAdapter-->>Middleware: Parse.Error("Database error")
Middleware->>Middleware: log original error
Middleware-->>Client: sanitized error response
else non-transient
StorageAdapter-->>Middleware: original error
Middleware-->>Client: original error response
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
mtrezza
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/middlewares.js (1)
376-384: Use INTERNAL_SERVER_ERROR to avoid returning 400 for internal failures.
handleParseErrorsmaps non-INTERNAL_SERVER_ERRORParse errors to 400; this catch represents an internal auth resolution failure and should stay 500.🛠️ Proposed fix
- next(new Parse.Error(Parse.Error.UNKNOWN_ERROR, 'Unknown error')); + next(new Parse.Error(Parse.Error.INTERNAL_SERVER_ERROR, 'Unknown error'));
Signed-off-by: Manuel <5673677+mtrezza@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/Adapters/Storage/Mongo/MongoStorageAdapter.js`:
- Around line 34-52: The if statements inside function isTransientError are
missing curly braces; update the function so each conditional block (the initial
null check "if (!error)", the transientErrorNames.include check "if
(transientErrorNames.includes(error.name))", and the nested label check "if
(error.hasErrorLabel('TransientTransactionError'))") uses braces around their
bodies to satisfy ESLint—i.e., wrap each single-line consequent in { ... } while
preserving current logic and return values.
🧹 Nitpick comments (1)
spec/MongoStorageAdapter.spec.js (1)
1067-1188: Add cleanup calls to prevent connection leaks.These tests call
adapter.connect()but don't calladapter.handleShutdown()afterwards. Other tests in this file (e.g., lines 863, 895, 998) follow the pattern of shutting down after testing. This could cause connection leaks during test runs.♻️ Proposed fix for one test (apply similarly to others)
it('should transform MongoWaitQueueTimeoutError to Parse.Error.INTERNAL_SERVER_ERROR', async () => { const adapter = new MongoStorageAdapter({ uri: databaseURI }); await adapter.connect(); // Create a mock error with the MongoWaitQueueTimeoutError name const mockError = new Error('Timed out while checking out a connection from connection pool'); mockError.name = 'MongoWaitQueueTimeoutError'; try { adapter.handleError(mockError); fail('Expected handleError to throw'); } catch (error) { expect(error instanceof Parse.Error).toBe(true); expect(error.code).toBe(Parse.Error.INTERNAL_SERVER_ERROR); expect(error.message).toBe('Database error'); + } finally { + await adapter.handleShutdown(); } });Alternatively, consider using a
beforeEach/afterEachpattern for the adapter lifecycle within this describe block.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## alpha #10020 +/- ##
==========================================
+ Coverage 92.12% 92.54% +0.42%
==========================================
Files 190 190
Lines 15477 15489 +12
Branches 176 176
==========================================
+ Hits 14258 14335 +77
+ Misses 1203 1142 -61
+ Partials 16 12 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
# [9.2.0-alpha.2](9.2.0-alpha.1...9.2.0-alpha.2) (2026-01-24) ### Bug Fixes * MongoDB timeout errors unhandled and potentially revealing internal data ([#10020](#10020)) ([1d3336d](1d3336d))
|
🎉 This change has been released in version 9.2.0-alpha.2 |
2 participants
Pull Request
Issue
If MongoDB driver timeout parameters are set, and a timeout occurs, the error crashes with an uncaught exception, like this:
Original code:
Crashing on database connection errors could be intentional, but wrapping it in
Parse.Errorsuggests the intent was to send an error response to the client, not to crash.Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.