fix: uncaught exception in case of missing source #747
Conversation
WalkthroughThe pull request focuses on enhancing the error handling and robustness of the Changes
Sequence DiagramsequenceDiagram
participant Middleware
participant Request
participant Source
Middleware->>Source: Check source exists
alt Source exists
Source-->>Middleware: Retrieve start date
Middleware->>Middleware: Validate start date
else Source is undefined
Middleware->>Middleware: Set startDate to undefined
Middleware->>Request: Proceed without setting notices
end
Possibly related PRs
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||
![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
🧹 Nitpick comments (2)
src/middleware/datasetOverview.middleware.js (1)
129-134: The fix correctly handles missing source, but could be more descriptiveThe changes properly address the uncaught exception when source is missing, but we could improve the error handling further.
Consider enhancing the error logging and restructuring the checks:
- const startDate = source ? new Date(source.startDate) : undefined - - if (!startDate || startDate.toString() === 'Invalid Date') { - logger.warn('Invalid start date encountered', { - type: types.DataValidation, - startDate: source?.startDate - }) + if (!source) { + logger.warn('Source object is missing', { + type: types.DataValidation, + sourceKey + }) + return next() + } + + const startDate = new Date(source.startDate) + if (startDate.toString() === 'Invalid Date') { + logger.warn('Invalid start date format', { + type: types.DataValidation, + startDate: source.startDate + }) return next() }test/unit/middleware/datasetOverview.middleware.test.js (1)
207-213: Test coverage could be more comprehensiveThe test correctly verifies the handling of missing source, but we could enhance the coverage.
Consider adding these additional test cases:
it('should handle source without startDate gracefully', () => { const reqWithEmptySource = { ...req, foobar: {} // source without startDate } setNoticesFromSourceKey('foobar')(reqWithEmptySource, res, () => {}) expect(reqWithEmptySource.notice).toBeUndefined() }) it('should handle invalid startDate format gracefully', () => { const reqWithInvalidDate = { ...req, foobar: { startDate: 'invalid-date' } } setNoticesFromSourceKey('foobar')(reqWithInvalidDate, res, () => {}) expect(reqWithInvalidDate.notice).toBeUndefined() })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/middleware/datasetOverview.middleware.js(1 hunks)test/unit/middleware/datasetOverview.middleware.test.js(1 hunks)
What type of PR is this? (check all applicable)
Description
Sometimes the 'source' in request object is not present and we end up accessing a member (
startDate) inundefinedvalue.Added/updated tests?
Summary by CodeRabbit
Bug Fixes
Tests