Skip to content

Enable detekt rule: EmptyCatchBlock#315

Open
younlll wants to merge 3 commits intomoneat-io:developfrom
younlll:feature/enable-detekt-empty-catch-block
Open

Enable detekt rule: EmptyCatchBlock#315
younlll wants to merge 3 commits intomoneat-io:developfrom
younlll:feature/enable-detekt-empty-catch-block

Conversation

@younlll
Copy link
Copy Markdown

@younlll younlll commented Mar 27, 2026

Description

Enable the EmptyCatchBlock detekt rule in backend/detekt.yml by setting active: true.
Ran ./gradlew detekt and ./gradlew detektFormat — no existing violations found, maxIssues: 0 maintained.

Related Issue

Fixes #307

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the CONTRIBUTING guide
  • I have signed the CLA
  • My code follows the project's coding conventions
  • I have added tests that cover my changes (if applicable)
  • All new and existing tests pass
  • I have addressed all code review feedback
  • I have joined Discord and notified the team that this PR is ready for review
  • I have updated documentation (if applicable)

Summary by CodeRabbit

  • Chores

    • Enabled a stricter code-quality check for empty catch blocks and standardized ignored-exception handling across the backend.
    • Replaced empty catch bodies with explicit "intentionally ignored" comments for clarity and consistency.
  • Notes

    • No user-facing changes; runtime behavior remains unchanged.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 27, 2026

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 27, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3fb9d08d-4926-4f81-8843-9f39c862d52d

📥 Commits

Reviewing files that changed from the base of the PR and between 90a0d37 and 600ea9c.

📒 Files selected for processing (1)
  • backend/src/main/kotlin/com/moneat/logs/routes/LogRoutes.kt
✅ Files skipped from review due to trivial changes (1)
  • backend/src/main/kotlin/com/moneat/logs/routes/LogRoutes.kt

📝 Walkthrough

Walkthrough

Enabled the Detekt rule empty-blocks.EmptyCatchBlock in backend/detekt.yml and replaced several previously-empty catch blocks across backend code with an explicit // intentionally ignored comment; no runtime behavior or control flow was altered.

Changes

Cohort / File(s) Summary
Detekt configuration
backend/detekt.yml
Set empty-blocks.EmptyCatchBlock.active from falsetrue.
Integration test
backend/src/integrationTest/kotlin/com/moneat/integration/LogQueryParserIntegrationTest.kt
Replaced an empty catch with a catch containing // intentionally ignored in waitForClickHouse().
Alerting service
backend/src/main/kotlin/com/moneat/dashboards/services/DashboardAlertService.kt
Added // intentionally ignored inside a previously-empty catch around Redis throttle-state write.
Log parsing
backend/src/main/kotlin/com/moneat/logs/services/LogService.kt, backend/src/main/kotlin/com/moneat/logs/routes/LogRoutes.kt
Inserted // intentionally ignored comments into several previously-empty catch blocks in JSON-line parsing and SSE unsubscribe cleanup; semantics unchanged.
Uptime checks
backend/src/main/kotlin/com/moneat/uptime/services/UptimeCheckExecutor.kt
Added // intentionally ignored to an empty catch around header decoding in checkHttp().

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

"I nibble on code with careful paws,
Empty catches now show their cause.
A tiny comment, polite and light,
Keeps detekt calm — hop, delight! 🐇"

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: enabling the detekt rule EmptyCatchBlock in the configuration file.
Linked Issues check ✅ Passed The PR fully addresses issue #307 objectives: enabled EmptyCatchBlock rule, fixed all violations with intentional comments, and maintains CI compliance.
Out of Scope Changes check ✅ Passed All changes are scoped to enabling EmptyCatchBlock and fixing its violations; no unrelated modifications present.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@younlll
Copy link
Copy Markdown
Author

younlll commented Mar 27, 2026

I have read the CLA Document and I hereby sign the CLA

AdrianLeeElder added a commit that referenced this pull request Mar 27, 2026
Copy link
Copy Markdown

@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

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@backend/detekt.yml`:
- Around line 82-83: Several empty catch blocks exist that will break the new
EmptyCatchBlock rule; locate the empty catches in the classes mentioned
(LogQueryParserIntegrationTest, UptimeCheckExecutor, LogService, LogRoutes,
DashboardAlertService) and replace each empty catch with proper handling: either
log the exception with context via the project's logger, rethrow or wrap it in a
meaningful exception, perform required cleanup, or—if truly intentional—add a
concise comment and apply a targeted suppression (e.g.,
`@Suppress`("EmptyCatchBlock")) on that specific catch; ensure you update the
specific methods containing those catches (test method in
LogQueryParserIntegrationTest, the executor method in UptimeCheckExecutor, the
failing handler methods in LogService, the route handler in LogRoutes, and the
alert method in DashboardAlertService) rather than broadly suppressing the rule.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4e7a2c7c-3d51-4d4c-b688-916d1f6fd734

📥 Commits

Reviewing files that changed from the base of the PR and between 1d1a3f1 and 8617a85.

📒 Files selected for processing (1)
  • backend/detekt.yml

Comment on lines 82 to +83
EmptyCatchBlock:
active: false
active: true
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Search for empty catch blocks in Kotlin source files and verify detekt passes

echo "=== Searching for empty catch blocks in Kotlin files ==="
# Search for catch blocks that might be empty (catch with underscore parameter and empty or whitespace-only body)
rg -n --type kotlin -U 'catch\s*\([^)]*\)\s*\{\s*\}' backend/src/

echo -e "\n=== Running detekt to verify no violations ==="
cd backend && ./gradlew detekt --console=plain 2>&1 | tail -20

Repository: moneat-io/moneat

Length of output: 1252


Address the 8 empty catch blocks before enabling this rule.

The codebase contains empty catch blocks that will cause the build to fail when this rule is enabled with maxIssues: 0:

  • LogQueryParserIntegrationTest.kt:108
  • UptimeCheckExecutor.kt:131
  • LogService.kt:592, 714, 928
  • LogRoutes.kt:491-492
  • DashboardAlertService.kt:359

These violations must be fixed before merging this change. Either remove the empty catch blocks, add logging/error handling, or suppress the rule for specific cases where they are intentional.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/detekt.yml` around lines 82 - 83, Several empty catch blocks exist
that will break the new EmptyCatchBlock rule; locate the empty catches in the
classes mentioned (LogQueryParserIntegrationTest, UptimeCheckExecutor,
LogService, LogRoutes, DashboardAlertService) and replace each empty catch with
proper handling: either log the exception with context via the project's logger,
rethrow or wrap it in a meaningful exception, perform required cleanup, or—if
truly intentional—add a concise comment and apply a targeted suppression (e.g.,
`@Suppress`("EmptyCatchBlock")) on that specific catch; ensure you update the
specific methods containing those catches (test method in
LogQueryParserIntegrationTest, the executor method in UptimeCheckExecutor, the
failing handler methods in LogService, the route handler in LogRoutes, and the
alert method in DashboardAlertService) rather than broadly suppressing the rule.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enable detekt rule: EmptyCatchBlock

1 participant