Remove And Delete error handling #1623
Conversation
A warning should be displayed instead of an error if the instance isn't found in the repository DB. trimble-oss#1621
There was a problem hiding this comment.
Pull Request Overview
This PR changes error handling when attempting to delete instances that don't exist in the repository database. Instead of throwing errors, the code now displays warnings when an instance isn't found, making the deletion process more user-friendly.
Key Changes:
- Modified exception handling to catch "Instance not found" SqlExceptions specifically and handle them as warnings instead of errors
- Updated
ShowExceptionDialogto safely handle null exception parameters
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| DBADashSharedGUI/CommonShared.cs | Added null-conditional operators to handle null exceptions in dialog display |
| DBADashServiceConfig/ServiceConfig.cs | Separated warnings from errors when deleting instances, displaying warnings for missing instances |
| DBADashConfig/Helper.cs | Changed missing instance handling from error to warning log entry |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { | ||
| SharedData.MarkInstanceDeleted(src.ConnectionID, dest.ConnectionString); | ||
| } | ||
| catch (SqlException ex) when (ex.Message == "Instance not found") |
There was a problem hiding this comment.
Comparing exception messages using string equality is fragile and can break if the message text changes. Consider checking the SQL error number instead, or creating a custom exception type for this specific case.
| catch (SqlException ex) when (ex.Message == "Instance not found") | |
| // Use error number instead of message for robustness. 50000 is the default for user-defined errors in SQL Server. | |
| catch (SqlException ex) when (ex.Number == 50000) |
| SharedData.MarkInstanceDeleted(connectionId, dest.ConnectionString); | ||
| } | ||
| } | ||
| catch (SqlException ex) when (ex.Message == "Instance not found") |
There was a problem hiding this comment.
Comparing exception messages using string equality is fragile and can break if the message text changes. Consider checking the SQL error number instead, or creating a custom exception type for this specific case.
| catch (SqlException ex) when (ex.Message == "Instance not found") | |
| // Replace "50001" with the actual error number used for "Instance not found" in your SQL code. | |
| catch (SqlException ex) when (ex.Number == 50001) |
1 participant
A warning should be displayed instead of an error if the instance isn't found in the repository DB. #1621