Emit unformatted output when formatting fails#2737
Conversation
|
Warning Rate limit exceeded@koxudaxi has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 17 minutes and 37 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe changes add graceful error handling for code formatting failures. When code formatting raises an exception, a warning is logged and unformatted code is emitted instead of failing completely. This includes wrapping formatting calls in try/except blocks in the base parser and adding new tests to verify this fallback behavior. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
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 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/datamodel_code_generator/parser/base.py (1)
2840-2847: Consider adjustingstacklevelfor better warning context.The warning uses
stacklevel=1, which points to this line inbase.pyrather than the user's call site. Usingstacklevel=2or higher would provide users with more helpful context about where in their code the formatting failure occurred.Additionally, this try/except pattern is duplicated at lines 2876-2883. Consider extracting a helper method like
_safe_format_code(code: str, formatter: CodeFormatter) -> strto reduce duplication.🔎 Proposed refactor to reduce duplication
+ def _safe_format_code(self, body: str, code_formatter: CodeFormatter) -> str: + """Apply code formatting with graceful fallback on error.""" + try: + return code_formatter.format_code(body) + except Exception as exc: # noqa: BLE001 + warn( + f"Failed to format code: {exc!r}. Emitting unformatted output.", + stacklevel=3, + ) + return body + def _generate_module_output(...): ... body = "\n".join(result) if config.code_formatter: - try: - body = config.code_formatter.format_code(body) - except Exception as exc: # noqa: BLE001 - warn( - f"Failed to format code: {exc!r}. Emitting unformatted output.", - stacklevel=1, - ) + body = self._safe_format_code(body, config.code_formatter)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/datamodel_code_generator/parser/base.pytests/main/test_main_general.py
🧰 Additional context used
🧬 Code graph analysis (2)
src/datamodel_code_generator/parser/base.py (1)
src/datamodel_code_generator/format.py (1)
format_code(272-291)
tests/main/test_main_general.py (2)
src/datamodel_code_generator/format.py (2)
CodeFormatter(162-332)PythonVersion(56-119)src/datamodel_code_generator/__init__.py (3)
generate(393-859)InputFileType(195-205)AllExportsScope(249-257)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: 3.11 on macOS
- GitHub Check: 3.14 on macOS
- GitHub Check: 3.11 on Windows
- GitHub Check: 3.10 on Windows
- GitHub Check: 3.12 on Windows
- GitHub Check: 3.14 on Windows
- GitHub Check: 3.13 on Windows
- GitHub Check: benchmarks
- GitHub Check: Analyze (python)
🔇 Additional comments (3)
tests/main/test_main_general.py (3)
21-21: LGTM!Import of
CodeFormatteris appropriate for mocking in the new fallback tests.
1095-1117: Comprehensive test coverage for formatting fallback.The test appropriately mocks the formatter to raise an exception, verifies the warning is emitted, and confirms that unformatted output is produced. Good coverage of the happy path for the fallback behavior.
1119-1140: Good coverage of init.py formatting fallback.This test effectively covers the second code path where formatting can fail (in
_generate_empty_init_exports). Using OpenAPI withAllExportsScope.Childrenis an appropriate way to exercise this path.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2737 +/- ##
=======================================
Coverage 99.36% 99.36%
=======================================
Files 83 83
Lines 12123 12165 +42
Branches 1458 1459 +1
=======================================
+ Hits 12046 12088 +42
Misses 45 45
Partials 32 32
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
CodSpeed Performance ReportMerging #2737 will not alter performanceComparing Summary
Footnotes
|
Fixes: #2315
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.