Add missing exception logging #130
Conversation
WalkthroughThis pull request enhances error logging across several modules of the vATIS.Desktop project. Changes include improved contextual messages for exceptions in methods such as updating IDS, audio requests, sandbox refreshing, ATIS station handling, and bot management. The modifications introduce additional catch blocks and integrate the Serilog logging library in select view models. The overall aim is to provide more detailed and structured error messages without altering the existing control flow of the application. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Method
participant ExceptionHandler as CatchBlock
participant Logger
Caller->>Method: Invoke operation
Note right of Method: Operation executes normally
Method-->>ExceptionHandler: Exception thrown
ExceptionHandler->>Logger: Log error with contextual message
Logger-->>ExceptionHandler: Error recorded
ExceptionHandler->>Caller: (Optionally rethrow exception)
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ Context from checks skipped due to timeout of 90000ms (7)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
vATIS.Desktop/TextToSpeech/TextToSpeechService.cs (1)
93-96: Consider adding more context to the error message.While the error logging is good, the message could be more descriptive to help with debugging.
- Log.Error(ex, "Error requesting audio"); + Log.Error(ex, "Error requesting audio for voice {Voice}", VoiceList.FirstOrDefault(v => v.Name == station.AtisVoice.Voice)?.Id ?? "default");vATIS.Desktop/Voice/Network/VoiceServerConnection.cs (2)
93-94: Consider adding more context to the error message.The error message could be more descriptive by including the callsign and bot details.
- Log.Error(ex, "AddOrUpdateBot failed."); + Log.Error(ex, "AddOrUpdateBot failed for callsign {Callsign}", callsign);
113-114: Consider adding more context to the error message.The error message could be more descriptive by including the callsign.
- Log.Error(ex, "RemoveBot failed."); + Log.Error(ex, "RemoveBot failed for callsign {Callsign}", callsign);vATIS.Desktop/Ui/ViewModels/MainWindowViewModel.cs (1)
350-350: Consider adding more context to the error message.The error message could be more descriptive by including the station details.
- Log.Error(ex, "Error Populating ATIS Station"); + Log.Error(ex, "Error Populating ATIS Station {StationId} {Identifier}", station.Id, station.Identifier);vATIS.Desktop/Ui/ViewModels/AtisConfiguration/SandboxViewModel.cs (1)
413-416: Consider adding more context to the error message and improve error handling.The error message could be more descriptive, and the error handling could be improved.
- Log.Error(ex, "HandleRefreshSandboxAtis Exception"); + Log.Error(ex, "Failed to refresh sandbox ATIS for station {StationId} {Identifier} with preset {PresetId}", + SelectedStation?.Id, SelectedStation?.Identifier, SelectedPreset?.Id); SandboxTextAtis = ""; SandboxSpokenTextAtis = ""; - throw; + SandboxTextAtis = "Error: " + ex.Message; + SandboxSpokenTextAtis = "Error: " + ex.Message;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
vATIS.Desktop/Atis/AtisBuilder.cs(1 hunks)vATIS.Desktop/TextToSpeech/TextToSpeechService.cs(1 hunks)vATIS.Desktop/Ui/ViewModels/AtisConfiguration/SandboxViewModel.cs(2 hunks)vATIS.Desktop/Ui/ViewModels/AtisStationViewModel.cs(12 hunks)vATIS.Desktop/Ui/ViewModels/MainWindowViewModel.cs(2 hunks)vATIS.Desktop/Voice/Network/VoiceServerConnection.cs(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: build (macos-latest)
- GitHub Check: build (windows-latest)
- GitHub Check: Analyze (cpp)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: Analyze (csharp)
- GitHub Check: Analyze (cpp)
- GitHub Check: Analyze (csharp)
🔇 Additional comments (11)
vATIS.Desktop/TextToSpeech/TextToSpeechService.cs (1)
91-92: LGTM! Clear comment for ignored exception.The comment clearly indicates that
OperationCanceledExceptionis intentionally ignored, which is a good practice for documenting exception handling decisions.vATIS.Desktop/Ui/ViewModels/MainWindowViewModel.cs (1)
23-23: LGTM! Added Serilog import.The import is correctly placed with other system imports.
vATIS.Desktop/Ui/ViewModels/AtisConfiguration/SandboxViewModel.cs (1)
21-21: LGTM! Added Serilog import.The import is correctly placed with other system imports.
vATIS.Desktop/Atis/AtisBuilder.cs (2)
140-143: LGTM! Improved error logging for HTTP request failures.The addition of a contextual message "HttpRequestException updating IDS" helps identify the source of the HTTP request failure.
144-148: LGTM! Added error logging before throwing the exception.Good practice to log the error before throwing the exception, as this ensures the error details are captured in logs even if the exception is caught and handled differently upstream.
vATIS.Desktop/Ui/ViewModels/AtisStationViewModel.cs (6)
857-860: LGTM! Added error logging for voice recording failures.Proper error logging for exceptions during voice ATIS recording.
889-894: LGTM! Added comprehensive error logging for network status changes.Good error logging coverage for:
- Voice server connection failures
- Voice server disconnection issues
- General network status change errors
Also applies to: 911-915, 927-930
997-1004: LGTM! Added error logging for network connection failures.Proper error logging for both specific connection failures and general network connect exceptions.
Also applies to: 1012-1023
1123-1140: LGTM! Added comprehensive error logging for METAR processing.Good error logging coverage for:
- METAR property update failures
- Text-to-speech ATIS generation errors
- General METAR response handling errors
Also applies to: 1200-1205, 1208-1219
1353-1364: LGTM! Added error logging for preset change failures.Proper error logging when handling ATIS preset changes.
1533-1537: LGTM! Added error logging for ATIS letter change failures.Good error logging coverage for both task-level and general ATIS letter change errors.
Also applies to: 1544-1555
Summary by CodeRabbit