Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+117
to
+120
| catch (Exception ex) | ||
| { | ||
| _logger.LogWarning(ex, "Error retrieving jobs for queue type {QueueType}", queueType); | ||
| } |
Check notice
Code scanning / CodeQL
Generic catch clause Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 minutes ago
General approach: replace the generic catch (Exception ex) with more specific catches for the exceptions we expect from SQL operations (e.g., SqlException, InvalidOperationException, DataException) and ensure we do not swallow cancellation-related exceptions such as OperationCanceledException or TaskCanceledException.
Best single fix for this snippet:
- In
GetJobsByQueueTypeAsync, replacecatch (Exception ex)with one or more specific catches for SQL/data errors, plus an optional filtered catch forTaskCanceledExceptionthat rethrows if the providedCancellationTokenwas signaled. - For example:
catch (SqlException ex)– log warning and return currentjobslist (preserves current behavior for SQL issues).catch (InvalidOperationException ex)and/orcatch (DataException ex)– likewise logged as warnings, since these are also typical data-access issues.- Optionally, allow
OperationCanceledExceptionand other non-data exceptions to bubble up, which is safer and more correct.
This can be implemented entirely within GetJobsByQueueTypeAsync in src/Microsoft.Health.Fhir.SqlServer/Features/Operations/SqlJobStatusService.cs. Necessary pieces:
- No new imports are needed:
SqlExceptionis already available viaMicrosoft.Data.SqlClient;, andDataExceptionis already available viausing System.Data;. - We just add multiple catch blocks in place of the single generic one.
Suggested changeset
1
src/Microsoft.Health.Fhir.SqlServer/Features/Operations/SqlJobStatusService.cs
| @@ -110,10 +110,18 @@ | ||
| jobs.Add(jobStatusInfo); | ||
| } | ||
| } | ||
| catch (Exception ex) | ||
| catch (SqlException ex) | ||
| { | ||
| _logger.LogWarning(ex, "Error retrieving jobs for queue type {QueueType}", queueType); | ||
| } | ||
| catch (InvalidOperationException ex) | ||
| { | ||
| _logger.LogWarning(ex, "Error retrieving jobs for queue type {QueueType}", queueType); | ||
| } | ||
| catch (DataException ex) | ||
| { | ||
| _logger.LogWarning(ex, "Error retrieving jobs for queue type {QueueType}", queueType); | ||
| } | ||
|
|
||
| return jobs; | ||
| } |
Copilot is powered by AI and may make mistakes. Always verify output.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Describe the changes in this PR.
Related issues
Addresses [issue #].
Testing
Describe how this change was tested.
FHIR Team Checklist
Semver Change (docs)
Patch|Skip|Feature|Breaking (reason)