Skip to content

CodeQL 5: refactor(ClinicalScheduler): catch specific exceptions in services #193

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rlorenzo wants to merge 3 commits into
base: main
Choose a base branch
from
Open

CodeQL 5: refactor(ClinicalScheduler): catch specific exceptions in services #193

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
125d2a9
refactor(ClinicalScheduler): catch specific exceptions in services
rlorenzo May 13, 2026
f5dbd34
chore(resharper): drop redundant namespace qualifiers in catch filters
rlorenzo May 13, 2026
39e16db
fix(ClinicalScheduler): correct catch filters in services
rlorenzo May 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion web/Areas/ClinicalScheduler/Services/InstructorScheduleService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using Microsoft.Data.SqlClient;
using Microsoft.EntityFrameworkCore;
using Viper.Classes.SQLContext;
using Viper.Models.CTS;
Expand Down Expand Up @@ -104,7 +105,7 @@ public async Task<List<InstructorSchedule>> GetInstructorScheduleAsync(
_logger.LogInformation("Found {Count} instructor schedules", schedules.Count);
return schedules;
}
catch (Exception ex)
catch (Exception ex) when (ex is DbUpdateException or SqlException or InvalidOperationException)
{
_logger.LogError(ex, "Error retrieving instructor schedules");
throw new InvalidOperationException("Failed to retrieve instructor schedules", ex);
Expand Down
12 changes: 6 additions & 6 deletions web/Areas/ClinicalScheduler/Services/PersonService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public PersonService(ILogger<PersonService> logger, ClinicalSchedulerContext con

return person;
}
catch (Exception ex)
catch (Exception ex) when (ex is DbUpdateException or SqlException or InvalidOperationException)
{
_logger.LogError(ex, "Error retrieving person data for MothraId: {MothraId}", LogSanitizer.SanitizeId(mothraId));
throw new InvalidOperationException($"Failed to retrieve person data for MothraId {LogSanitizer.SanitizeId(mothraId)}", ex);
Expand Down Expand Up @@ -139,7 +139,7 @@ public async Task<List<ClinicianYearSummary>> GetCliniciansByYearAsync(int year,
_logger.LogInformation("Found {ClinicianCount} clinicians for year {Year} with person names from vPerson view", clinicians.Count, year);
return clinicians;
}
catch (Exception ex)
catch (Exception ex) when (ex is DbUpdateException or SqlException or InvalidOperationException)
{
_logger.LogError(ex, "Error retrieving clinicians for year: {Year}", year);
throw new InvalidOperationException($"Failed to retrieve clinicians for grad year {year}. Check database connectivity and view permissions.", ex);
Expand Down Expand Up @@ -209,7 +209,7 @@ public async Task<List<ClinicianSummary>> GetCliniciansByGradYearRangeAsync(int
_logger.LogError(ex, "Database error retrieving clinicians for grad year range {StartYear}-{EndYear}", LogSanitizer.SanitizeYear(startGradYear), LogSanitizer.SanitizeYear(endGradYear));
throw new InvalidOperationException($"Database error retrieving clinicians for grad year range {LogSanitizer.SanitizeYear(startGradYear)}-{LogSanitizer.SanitizeYear(endGradYear)}", ex);
}
catch (Exception ex)
catch (Exception ex) when (ex is DbUpdateException or InvalidOperationException)
{
_logger.LogError(ex, "Error retrieving clinicians for grad year range {StartYear}-{EndYear}", LogSanitizer.SanitizeYear(startGradYear), LogSanitizer.SanitizeYear(endGradYear));
throw new InvalidOperationException($"Failed to retrieve clinicians for grad year range {LogSanitizer.SanitizeYear(startGradYear)}-{LogSanitizer.SanitizeYear(endGradYear)}", ex);
Expand Down Expand Up @@ -240,7 +240,7 @@ public async Task<List<string>> GetAllMothraIdsAsync(CancellationToken cancellat
_logger.LogInformation("Found {Count} unique MothraIds in Clinical Scheduler data", mothraIds.Count);
return mothraIds;
}
catch (Exception ex)
catch (Exception ex) when (ex is DbUpdateException or SqlException or InvalidOperationException)
{
_logger.LogError(ex, "Error retrieving unique MothraIds");
throw new InvalidOperationException("Failed to retrieve unique MothraIds from database", ex);
Expand Down Expand Up @@ -277,7 +277,7 @@ public async Task<List<ClinicianSummary>> GetAllActiveEmployeeAffiliatesAsync(Ca
_logger.LogDebug("Found {Count} active employee affiliates from AAUD", allAffiliates.Count);
return allAffiliates;
}
catch (Exception ex)
catch (Exception ex) when (ex is DbUpdateException or SqlException or InvalidOperationException)
{
_logger.LogError(ex, "Error retrieving active employee affiliates from AAUD");
throw new InvalidOperationException("Failed to retrieve active employee affiliates from AAUD database", ex);
Expand Down Expand Up @@ -325,7 +325,7 @@ public async Task<List<ClinicianSummary>> GetAllActiveEmployeeAffiliatesAsync(Ca

return clinician;
}
catch (Exception ex)
catch (Exception ex) when (ex is DbUpdateException or SqlException or InvalidOperationException)
{
_logger.LogError(ex, "Error retrieving clinician data for MothraId: {MothraId} from AAUD", LogSanitizer.SanitizeId(mothraId));
throw new InvalidOperationException($"Failed to retrieve clinician data for MothraId {LogSanitizer.SanitizeId(mothraId)} from AAUD", ex);
Expand Down
15 changes: 8 additions & 7 deletions web/Areas/ClinicalScheduler/Services/RotationService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using Microsoft.Data.SqlClient;
using Microsoft.EntityFrameworkCore;
using Viper.Areas.ClinicalScheduler.Extensions;
using Viper.Areas.ClinicalScheduler.Models.DTOs.Responses;
Expand Down Expand Up @@ -45,7 +46,7 @@ public async Task<List<RotationDto>> GetRotationsAsync(CancellationToken cancell
rotations.Count);
return rotations.Select(r => r.ToDto()).ToList();
}
catch (Exception ex)
catch (Exception ex) when (ex is DbUpdateException or SqlException or InvalidOperationException)
{
_logger.LogError(ex, "Error retrieving rotations from Clinical Scheduler");
throw new InvalidOperationException("Failed to retrieve rotations from Clinical Scheduler database", ex);
Expand Down Expand Up @@ -82,7 +83,7 @@ public async Task<List<RotationDto>> GetRotationsAsync(CancellationToken cancell

return rotation?.ToDto();
}
catch (Exception ex)
catch (Exception ex) when (ex is DbUpdateException or SqlException or InvalidOperationException)
{
_logger.LogError(ex, "Error retrieving rotation by ID: {RotationId}", rotationId);
throw new InvalidOperationException($"Failed to retrieve rotation with ID {rotationId}", ex);
Expand Down Expand Up @@ -124,7 +125,7 @@ public async Task<List<Rotation>> GetRotationsByCourseAsync(string courseNumber,

return rotations;
}
catch (Exception ex)
catch (Exception ex) when (ex is DbUpdateException or SqlException or InvalidOperationException)
{
_logger.LogError(ex, "Error retrieving rotations by course - CourseNumber: {CourseNumber}, SubjectCode: {SubjectCode}",
courseNumber, subjectCode);
Expand Down Expand Up @@ -154,7 +155,7 @@ public async Task<List<RotationDto>> GetRotationsByServiceAsync(int serviceId, C
_logger.LogInformation("Retrieved {Count} rotations for service ID: {ServiceId}", rotations.Count, serviceId);
return rotations.Select(r => r.ToDto()).ToList();
}
catch (Exception ex)
catch (Exception ex) when (ex is DbUpdateException or SqlException or InvalidOperationException)
{
_logger.LogError(ex, "Error retrieving rotations by service ID: {ServiceId}", serviceId);
throw new InvalidOperationException($"Failed to retrieve rotations for service ID {serviceId}", ex);
Expand All @@ -180,7 +181,7 @@ public async Task<List<ServiceDto>> GetServicesAsync(CancellationToken cancellat
_logger.LogInformation("Retrieved {Count} services from Clinical Scheduler", services.Count);
return services.Select(s => s.ToDto()).ToList();
}
catch (Exception ex)
catch (Exception ex) when (ex is DbUpdateException or SqlException or InvalidOperationException)
{
_logger.LogError(ex, "Error retrieving services from Clinical Scheduler");
throw new InvalidOperationException("Failed to retrieve services from Clinical Scheduler database", ex);
Expand Down Expand Up @@ -215,7 +216,7 @@ public async Task<List<ServiceDto>> GetServicesAsync(CancellationToken cancellat

return service?.ToDto();
}
catch (Exception ex)
catch (Exception ex) when (ex is DbUpdateException or SqlException or InvalidOperationException)
{
_logger.LogError(ex, "Error retrieving service by ID: {ServiceId}", serviceId);
throw new InvalidOperationException($"Failed to retrieve service with ID {serviceId}", ex);
Expand Down Expand Up @@ -265,7 +266,7 @@ public async Task<List<InstructorSchedule>> GetInstructorSchedulesByRotationAsyn

return schedules;
}
catch (Exception ex)
catch (Exception ex) when (ex is DbUpdateException or SqlException or InvalidOperationException)
{
_logger.LogError(ex, "Error retrieving instructor schedules for rotation ID: {RotationId}", rotationId);
throw new InvalidOperationException($"Failed to retrieve instructor schedules for rotation ID {rotationId}", ex);
Expand Down
7 changes: 4 additions & 3 deletions web/Areas/ClinicalScheduler/Services/ScheduleAuditService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using Microsoft.Data.SqlClient;
using Microsoft.EntityFrameworkCore;
using Viper.Classes.SQLContext;
using Viper.Models.ClinicalScheduler;
Expand Down Expand Up @@ -114,7 +115,7 @@ public async Task<List<ScheduleAudit>> GetInstructorScheduleAuditHistoryAsync(
.OrderByDescending(a => a.TimeStamp)
.ToListAsync(cancellationToken);
}
catch (Exception ex)
catch (Exception ex) when (ex is DbUpdateException or SqlException or InvalidOperationException)
{
_logger.LogError(ex, "Error retrieving audit history for instructor schedule {ScheduleId}", instructorScheduleId);
throw new InvalidOperationException($"Failed to retrieve audit history for instructor schedule {instructorScheduleId}. Please try again or contact support if the problem persists.", ex);
Expand All @@ -134,7 +135,7 @@ public async Task<List<ScheduleAudit>> GetRotationWeekAuditHistoryAsync(
.OrderByDescending(a => a.TimeStamp)
.ToListAsync(cancellationToken);
}
catch (Exception ex)
catch (Exception ex) when (ex is DbUpdateException or SqlException or InvalidOperationException)
{
_logger.LogError(ex, "Error retrieving audit history for rotation {RotationId}, week {WeekId}", rotationId, weekId);
throw new InvalidOperationException($"Failed to retrieve audit history for rotation {rotationId}, week {weekId}. Please try again or contact support if the problem persists.", ex);
Expand Down Expand Up @@ -179,7 +180,7 @@ private async Task<ScheduleAudit> CreateAuditEntryAsync(

return auditEntry;
}
catch (Exception ex)
catch (Exception ex) when (ex is DbUpdateException or SqlException or InvalidOperationException)
{
_logger.LogError(ex, "Error creating audit entry for action {Action}, {MothraId} on rotation {RotationId}, week {WeekId}",
action, LogSanitizer.SanitizeId(mothraId), rotationId, weekId);
Expand Down
28 changes: 18 additions & 10 deletions web/Areas/ClinicalScheduler/Services/ScheduleEditService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using Microsoft.Data.SqlClient;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Options;
using Viper.Areas.ClinicalScheduler.EmailTemplates.Models;
Expand Down Expand Up @@ -46,7 +47,7 @@
_emailTemplateRenderer = emailTemplateRenderer;
}

public async Task<List<InstructorSchedule>> AddInstructorAsync(

Check warning on line 50 in web/Areas/ClinicalScheduler/Services/ScheduleEditService.cs

View workflow job for this annotation

GitHub Actions / Backend Tests 

'AddInstructorAsync' has a cyclomatic complexity of '37'. Rewrite or refactor the code to decrease its complexity below '26'. (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1502)
string? mothraId,
int rotationId,
int[] weekIds,
Expand Down Expand Up @@ -216,7 +217,9 @@
}
#pragma warning restore S3267
}
#pragma warning disable CA1031 // Intentional broad catch: post-transaction work (email/audit notifications) must not roll back the successful database changes above.
catch (Exception postTransactionEx)
#pragma warning restore CA1031
{
// Log warning but don't fail the operation - the database changes were successful
_logger.LogWarning(postTransactionEx, "Post-transaction operations failed for instructor {MothraId} in rotation {RotationId}, but database changes were successful",
Expand All @@ -238,7 +241,7 @@
// Re-throw InvalidOperationException without wrapping (includes "already scheduled" messages)
throw;
}
catch (Exception saveEx)
catch (Exception saveEx) when (saveEx is DbUpdateException or SqlException)
{
_logger.LogError(saveEx, "Database save failed for MothraId='{MothraId}', RotationId={RotationId}, WeekIds=[{WeekIds}]",
LogSanitizer.SanitizeId(mothraId), rotationId, string.Join(",", weekIds));
Expand Down Expand Up @@ -332,7 +335,9 @@
await HandlePrimaryEvaluatorRemovalAsync(schedule, currentUser.MothraId, cancellationToken);
}
}
#pragma warning disable CA1031 // Intentional broad catch: post-transaction work (email/audit notifications) must not roll back the successful database changes above.
catch (Exception postTransactionEx)
#pragma warning restore CA1031
{
// Log warning but don't fail the operation - the database changes were successful
_logger.LogWarning(postTransactionEx, "Post-transaction operations failed for instructor removal {ScheduleId}, but database changes were successful",
Expand All @@ -354,7 +359,7 @@
// Re-throw InvalidOperationException without wrapping (includes "Cannot remove primary evaluator" message)
throw;
}
catch (Exception ex)
catch (Exception ex) when (ex is DbUpdateException or SqlException)
{
_logger.LogError(ex, "Error removing instructor schedule {ScheduleId}", instructorScheduleId);
throw new InvalidOperationException($"Failed to remove instructor schedule. Please try again or contact support if the problem persists.", ex);
Expand Down Expand Up @@ -451,7 +456,9 @@
await HandlePrimaryEvaluatorRemovalAsync(schedule, currentUser.MothraId, cancellationToken, null, requiresPrimaryEvaluator);
}
}
#pragma warning disable CA1031 // Intentional broad catch: post-transaction work (email/audit notifications) must not roll back the successful database changes above.
catch (Exception postTransactionEx)
#pragma warning restore CA1031
{
// Log warning but don't fail the operation - the database changes were successful
_logger.LogWarning(postTransactionEx, "Post-transaction operations failed for primary evaluator update {ScheduleId}, but database changes were successful",
Expand All @@ -463,7 +470,7 @@

return (true, previousPrimaryName);
}
catch (Exception ex)
catch (Exception ex) when (ex is DbUpdateException or SqlException or InvalidOperationException)
{
_logger.LogError(ex, "Error setting primary evaluator for instructor schedule {ScheduleId} to {IsPrimary}", instructorScheduleId, isPrimary);
throw new InvalidOperationException($"Failed to update primary evaluator status. Please try again or contact support if the problem persists.", ex);
Expand All @@ -488,7 +495,7 @@
// All instructors can now be removed, including primary evaluators
return true;
}
catch (Exception ex)
catch (Exception ex) when (ex is DbUpdateException or SqlException or InvalidOperationException)
{
_logger.LogError(ex, "Error checking if instructor schedule {ScheduleId} can be removed", instructorScheduleId);
return false;
Expand Down Expand Up @@ -530,7 +537,7 @@

return await query.ToListAsync(cancellationToken);
}
catch (Exception ex)
catch (Exception ex) when (ex is DbUpdateException or SqlException or InvalidOperationException)
{
_logger.LogError(ex, "Error checking other rotation schedules for {MothraId} on weeks {WeekIds} for grad year {GradYear}",
LogSanitizer.SanitizeId(mothraId), string.Join(",", weekIds), LogSanitizer.SanitizeYear(gradYear));
Expand Down Expand Up @@ -558,7 +565,7 @@
.Where(s => s.RotationId == rotationId && weekIds.Contains(s.WeekId))
.ToListAsync(cancellationToken);
}
catch (Exception ex)
catch (Exception ex) when (ex is DbUpdateException or SqlException or InvalidOperationException)
{
_logger.LogError(ex, "Error getting scheduled instructors for rotation {RotationId} on weeks {WeekIds}", rotationId, string.Join(",", weekIds));
throw new InvalidOperationException($"Failed to retrieve scheduled instructors. Please try again or contact support if the problem persists.", ex);
Expand Down Expand Up @@ -661,7 +668,7 @@
}
}
}
catch (Exception ex)
catch (Exception ex) when (ex is DbUpdateException or SqlException or InvalidOperationException)
{
_logger.LogWarning(ex, "Could not retrieve instructor name for {MothraId} in email notification", LogSanitizer.SanitizeId(schedule.MothraId));
}
Expand Down Expand Up @@ -694,7 +701,7 @@
weekNumber = weekGradYear.WeekNum.ToString();
}
}
catch (Exception ex)
catch (Exception ex) when (ex is DbUpdateException or SqlException or InvalidOperationException)
{
_logger.LogWarning(ex, "Could not retrieve week number for {WeekId} in email notification", schedule.WeekId);
}
Expand All @@ -719,7 +726,7 @@
: "";
}
}
catch (Exception ex)
catch (Exception ex) when (ex is DbUpdateException or SqlException or InvalidOperationException)
{
_logger.LogWarning(ex, "Could not retrieve modifier information for {MothraId} in email notification", modifiedByMothraId);
}
Expand Down Expand Up @@ -770,9 +777,10 @@
_logger.LogInformation("No notification recipients configured for Primary Evaluator Removal; skipped email. Rotation={RotationName}, Week={WeekNumber}", rotationName, weekNumber);
}
}
#pragma warning disable CA1031 // Intentional broad catch: email notification is secondary to the schedule removal; any failure must be logged but not propagated.
catch (Exception ex)
#pragma warning restore CA1031
{
// Log error but don't fail the transaction - email is secondary to the schedule removal
_logger.LogError(ex, "Failed to send primary evaluator removal notification for {MothraId} from rotation {RotationId} week {WeekId} (rotation: {RotationName})",
LogSanitizer.SanitizeId(schedule.MothraId), schedule.RotationId, schedule.WeekId, schedule.Rotation?.Name ?? "Unknown");
}
Expand Down
Loading
Loading