CodeQL 6: refactor: catch specific exceptions across CTS/RAPS/shared#194
CodeQL 6: refactor: catch specific exceptions across CTS/RAPS/shared#194rlorenzo wants to merge 2 commits into
Conversation
CodeQL cs/catch-of-all-exceptions: same when-filter pattern as #193, applied across the remaining areas (~46 sites in 28 files): - CTS controllers (9 files) - ClinicalScheduler controllers (4 files) - RAPS controllers and services (5 files) - Computing service, shared web/Classes, web/Classes/Utilities (7 files) - web/Services/EmailService.cs, web/ViteProxyHelpers.cs, web/Program.cs Filter is restricted to DbUpdateException, SqlException, InvalidOperationException, OperationCanceledException (plus IOException for ViteProxyHelpers and FormatException/ArgumentException for the MailAddress email validator); anything outside that set now propagates. Two intentional broad catches kept with #pragma warning disable CA1031: - web/Program.cs top-level startup must catch any exception to log fatal before rethrowing. Bare `catch { }` in RoleMembersController (VMACS JSON parse) and BiorenderStudentLookup (MailAddress validation) tightened to the specific exceptions those calls actually throw.
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
Bundle ReportBundle size has no change ✅ |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #194 +/- ##
==========================================
- Coverage 42.96% 42.95% -0.02%
==========================================
Files 877 877
Lines 51468 51468
Branches 4802 4802
==========================================
- Hits 22113 22107 -6
- Misses 28831 28837 +6
Partials 524 524
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
…ndant qualifiers The codeql/6 batch script converted bare 'catch (Exception)' blocks to 'catch (Exception) when (true /* placeholder */)' to satisfy CodeQL. ReSharper rightly flagged that as CS7095 (filter is a constant). Replaced 14 of those with the standard when-filter pattern 'catch (Exception ex) when (ex is DbUpdateException or SqlException or InvalidOperationException or OperationCanceledException)' used in the rest of the PR. Also dropped Microsoft.EntityFrameworkCore. and Microsoft.Data.SqlClient. qualifiers in catch filters across 26 files; added the corresponding 'using' directives where missing.
rlorenzo
force-pushed
the
codeql/6-other-exceptions
branch
from
f559040 to
fd88995
Compare
2 participants
Summary
Closes ~44 of 46 remaining
cs/catch-of-all-exceptionsalerts. Same when-filter pattern as #193, applied across the rest of the codebase.Files changed (28)
Filter is
DbUpdateException∨SqlException∨InvalidOperationException∨OperationCanceledExceptionfor most catches, withIOExceptionadded in ViteProxyHelpers (proxy-and-file-fallback) andFormatException/ArgumentExceptionfor the MailAddress email validator.Bare
catch { }inRoleMembersController(VMACS JSON parse) andBiorenderStudentLookup(MailAddress validation) tightened to the specific exceptions those calls actually throw.Why 2 broad catches are kept
web/Program.cs:527- top-level startup must catch any exception to log fatal before rethrowing. Marked#pragma warning disable CA1031.The 4 ClinicalScheduler/Services post-transaction catches kept broad in #193 remain so on this branch. Test files (test/CTS/AssessmentControllerTest.cs helpers, test/ClinicalScheduler/EmailNotificationTest.cs diagnostic catches) are intentionally left as broad catches - they are test infrastructure, not production code.
Context
Sixth in the
CodeQL N:cleanup series (after #189, #190, #191, #192, #193).Test plan
npm run test:backend- 1946 tests passingnpm run verify:build- clean (0 errors)