feat: Add logLevels support to suppress deprecation warnings - #10599
feat: Add logLevels support to suppress deprecation warnings#10599anaghroy wants to merge 4 commits into
Conversation
… deprecation warnings
|
I will reformat the title to use the proper commit message syntax. |
parse-github-assistant
Bot
changed the title
|
🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review. Tip
Note Please respond to review comments from AI agents just like you would to comments from a human reviewer. Let the reviewer resolve their own comments, unless they have reviewed and accepted your commit, or agreed with your explanation for why the feedback was incorrect. Caution Pull requests must be written using an AI agent with human supervision. Pull requests written entirely by a human will likely be rejected, because of lower code quality, higher review effort and the higher risk of introducing bugs. Please note that AI review comments on this pull request alone do not satisfy this requirement. Our CI and AI review are safeguards, not development tools. If many issues are flagged, rethink your development approach. Invest more effort in planning and design rather than using review cycles to fix low-quality code. |
📝 WalkthroughWalkthroughAdds configurable deprecation log levels, including global and per-option suppression. Deprecator now passes options into log dispatch, supports silent and other logger levels, and retains warning fallback behavior. Configuration typing, definitions, documentation, and suppression tests are updated. ChangesConfigurable deprecation logging
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (6 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
anaghroy
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
spec/Deprecator.spec.js (1)
38-57: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert that
silentsuppresses every logger method.These tests spy only on
warn, so a regression that logs throughinfoorerrorwould still pass. Spy on all supported logger methods (or inject a logger double) and assert that none were called.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@spec/Deprecator.spec.js` around lines 38 - 57, Update both silent deprecation tests around reconfigureServer to monitor every supported logger method, not only logger.warn, and assert that none are invoked. Keep the existing separate global and option-specific silent configurations and deprecation setup unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Deprecator/Deprecator.js`:
- Around line 121-146: Restrict the configuration-derived level in the
deprecation logging flow to the documented logger levels plus the special
`silent` value before dynamic dispatch. Update the `logger[level]` check to
reject inherited or unsupported callable properties, while preserving the early
return for `silent` and the existing `logger.warn(output)` fallback for invalid
levels.
---
Nitpick comments:
In `@spec/Deprecator.spec.js`:
- Around line 38-57: Update both silent deprecation tests around
reconfigureServer to monitor every supported logger method, not only
logger.warn, and assert that none are invoked. Keep the existing separate global
and option-specific silent configurations and deprecation setup unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a329c504-3d40-463f-9838-d7f22ff43b1c
📒 Files selected for processing (4)
spec/Deprecator.spec.jssrc/Deprecator/Deprecator.jssrc/Options/Definitions.jssrc/Options/index.js
| // Determine the log level | ||
| const logLevels = (options && options.logLevels) || {}; | ||
| let level = 'warn'; | ||
| if (key && logLevels[`deprecation_${key}`]) { | ||
| level = logLevels[`deprecation_${key}`]; | ||
| } else if (logLevels['deprecation']) { | ||
| level = logLevels['deprecation']; | ||
| } | ||
|
|
||
| if (level === 'silent') { | ||
| return; | ||
| } | ||
|
|
||
| // Compose message | ||
| let output = `DeprecationWarning: The Parse Server ${type} '${key}' `; | ||
| output += changeNewKey != null ? `is deprecated and will be ${keyAction} in a future version.` : ''; | ||
| output += changeNewDefault | ||
| ? `default will change to '${changeNewDefault}' in a future version.` | ||
| : ''; | ||
| output += solution ? ` ${solution}` : ''; | ||
| logger.warn(output); | ||
|
|
||
| if (typeof logger[level] === 'function') { | ||
| logger[level](output); | ||
| } else { | ||
| logger.warn(output); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Allow-list log levels before dynamic dispatch.
level is configuration-controlled, but typeof logger[level] === 'function' also accepts inherited callable properties such as constructor. Calling logger.constructor(output) can throw during startup, while values like toString silently avoid logging. Restrict dispatch to the documented logger levels plus the special silent value, then retain the warning fallback for invalid values.
Proposed fix
+const validLevels = new Set(['error', 'warn', 'info', 'verbose', 'debug', 'silly']);
+
- if (typeof logger[level] === 'function') {
+ if (validLevels.has(level) && typeof logger[level] === 'function') {
logger[level](output);
} else {
logger.warn(output);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Determine the log level | |
| const logLevels = (options && options.logLevels) || {}; | |
| let level = 'warn'; | |
| if (key && logLevels[`deprecation_${key}`]) { | |
| level = logLevels[`deprecation_${key}`]; | |
| } else if (logLevels['deprecation']) { | |
| level = logLevels['deprecation']; | |
| } | |
| if (level === 'silent') { | |
| return; | |
| } | |
| // Compose message | |
| let output = `DeprecationWarning: The Parse Server ${type} '${key}' `; | |
| output += changeNewKey != null ? `is deprecated and will be ${keyAction} in a future version.` : ''; | |
| output += changeNewDefault | |
| ? `default will change to '${changeNewDefault}' in a future version.` | |
| : ''; | |
| output += solution ? ` ${solution}` : ''; | |
| logger.warn(output); | |
| if (typeof logger[level] === 'function') { | |
| logger[level](output); | |
| } else { | |
| logger.warn(output); | |
| } | |
| // Determine the log level | |
| const logLevels = (options && options.logLevels) || {}; | |
| const validLevels = new Set(['error', 'warn', 'info', 'verbose', 'debug', 'silly']); | |
| let level = 'warn'; | |
| if (key && logLevels[`deprecation_${key}`]) { | |
| level = logLevels[`deprecation_${key}`]; | |
| } else if (logLevels['deprecation']) { | |
| level = logLevels['deprecation']; | |
| } | |
| if (level === 'silent') { | |
| return; | |
| } | |
| // Compose message | |
| let output = `DeprecationWarning: The Parse Server ${type} '${key}' `; | |
| output += changeNewKey != null ? `is deprecated and will be ${keyAction} in a future version.` : ''; | |
| output += changeNewDefault | |
| ? `default will change to '${changeNewDefault}' in a future version.` | |
| : ''; | |
| output += solution ? ` ${solution}` : ''; | |
| if (validLevels.has(level) && typeof logger[level] === 'function') { | |
| logger[level](output); | |
| } else { | |
| logger.warn(output); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Deprecator/Deprecator.js` around lines 121 - 146, Restrict the
configuration-derived level in the deprecation logging flow to the documented
logger levels plus the special `silent` value before dynamic dispatch. Update
the `logger[level]` check to reject inherited or unsupported callable
properties, while preserving the early return for `silent` and the existing
`logger.warn(output)` fallback for invalid levels.
1 participant
Pull Request
Issue
Closes #10584
Approach
Following the maintainers' suggestions, this PR integrates the suppression of deprecation warnings directly into the existing
logLevelsconfiguration inParseServerOptions.Developers can now suppress all deprecation warnings globally: