Fix Boilerplate's logging issues (#9098) #9099
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes in this pull request primarily enhance the logging capabilities across various components of the application. Key modifications include the introduction of a logger for authentication state changes, updates to the telemetry context handling, and refinements in exception handling logic. Additionally, logging configurations have been updated in JSON files to improve the granularity of logs related to specific services. Overall, the changes focus on improving the clarity and maintainability of logging and error handling without altering the existing control flows. Changes
Assessment against linked issues
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: 1
🧹 Outside diff range and nitpick comments (14)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Web/Services/WebExceptionHandler.cs (1)
7-9: Consider adding trace logging for ignored exceptions.The implementation is correct, but adding trace-level logging for ignored exceptions would help with debugging and monitoring.
if (IgnoreException(exception)) + { + Logger.LogTrace(exception, "Exception ignored by {Handler}: {ExceptionType}", + nameof(WebExceptionHandler), exception.GetType().Name); return; + }src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/appsettings.Development.json (1)
Line range hint
1-18: LGTM! Configuration structure follows best practices.The JSON structure is well-organized and properly nested. The use of conditional compilation directive
//#if (appInsights == true)appropriately encapsulates the Application Insights configuration for template generation.Consider adding a comment in the template documentation explaining the difference between these two logging providers and when each should be used.
🧰 Tools
🪛 Biome
[error] 11-11: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 12-12: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 12-12: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 12-16: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Maui/Services/MauiExceptionHandler.cs (1)
14-16: LGTM! Consider adding debug logging for ignored exceptions.The early return pattern is well implemented and aligns with the class's responsibility. However, for better diagnostics, consider logging ignored exceptions at debug level.
if (IgnoreException(exception)) + { + Logger.LogDebug(exception, "Exception ignored by MauiExceptionHandler"); return; + }src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Windows/Services/WindowsExceptionHandler.cs (1)
13-15: LGTM! Consider adding documentation about exception filtering.The addition of the exception filtering logic is clean and aligns well with the implementations in other platform-specific handlers. However, it would be beneficial to document which exceptions are being filtered out.
Add XML documentation to explain the filtering behavior:
protected override void Handle(Exception exception, Dictionary<string, object> parameters) { + // Skip handling of specific exceptions (e.g., TaskCanceledException) as defined in the base class if (IgnoreException(exception)) return; base.Handle(exception, parameters); }src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Windows/Program.cs (1)
Line range hint
28-45: Enhance error logging for update failures.The error handling is appropriate, but consider adding more detailed logging to help diagnose update failures:
- Log the specific stage of the update process where the failure occurred
- Include relevant update information in the error context
Consider wrapping the update stages with more detailed error context:
try { var windowsUpdateSettings = services.GetRequiredService<ClientWindowsSettings>().WindowsUpdate; if (string.IsNullOrEmpty(windowsUpdateSettings?.FilesUrl)) { + services.GetRequiredService<ILogger>().LogWarning("Update check skipped - FilesUrl not configured"); return; } var updateManager = new UpdateManager(windowsUpdateSettings.FilesUrl); + services.GetRequiredService<ILogger>().LogInformation("Checking for updates from {Url}", windowsUpdateSettings.FilesUrl); var updateInfo = await updateManager.CheckForUpdatesAsync(); if (updateInfo is not null) { + services.GetRequiredService<ILogger>().LogInformation("Update found: {Version}", updateInfo.Version); await updateManager.DownloadUpdatesAsync(updateInfo); if (windowsUpdateSettings.AutoReload) { + services.GetRequiredService<ILogger>().LogInformation("Applying update and restarting"); updateManager.ApplyUpdatesAndRestart(updateInfo, args); } } } catch (Exception exp) { + services.GetRequiredService<ILogger>().LogError(exp, "Update process failed"); services.GetRequiredService<IExceptionHandler>().Handle(exp); }src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Services/Contracts/ITelemetryContext.cs (2)
49-72: Consider improving parameter handling in ToDictionary method.The implementation looks good overall, but there are a couple of suggestions for improvement:
- The parameter mutation on line 51 could be unexpected. Consider separating the initialization:
- var data = new Dictionary<string, object?>(additionalParameters ??= []) + additionalParameters ??= []; + var data = new Dictionary<string, object?>(additionalParameters)
- Consider documenting the parameter mutation behavior in XML comments.
53-60: Consider adding null checks for string properties.The string properties (OS, AppVersion, UserAgent, etc.) are added to the dictionary without null checks. While nullable string properties will be added as null values, you might want to consider excluding null values to reduce dictionary size and improve clarity.
Here's a suggested improvement:
- { nameof(OS), OS }, - { nameof(AppVersion), AppVersion }, - { nameof(UserAgent), UserAgent }, - { nameof(TimeZone), TimeZone }, - { nameof(Culture), Culture }, + { nameof(OS), OS }.WhereValueNotNull(), + { nameof(AppVersion), AppVersion }.WhereValueNotNull(), + { nameof(UserAgent), UserAgent }.WhereValueNotNull(), + { nameof(TimeZone), TimeZone }.WhereValueNotNull(), + { nameof(Culture), Culture }.WhereValueNotNull(),You'll need to add an extension method:
public static KeyValuePair<string, T?> WhereValueNotNull<T>(this KeyValuePair<string, T?> kvp) where T : class { return kvp.Value is not null ? kvp : default; }src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Maui/Services/MauiLocalHttpServer.cs (1)
43-48: Consider enriching the HTTP error context.While the current implementation captures basic HTTP error details, consider adding more context that could be valuable for debugging:
exceptionHandler.Handle(new HttpRequestException(exception.Message), new Dictionary<string, object?>() { { "StatusCode" , exception.StatusCode }, { "RequestUri" , context.Request.Url }, + { "HttpMethod", context.Request.HttpMethod }, + { "UserAgent", context.Request.UserAgent }, + { "RemoteEndPoint", context.Request.RemoteEndPoint }, });src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Windows/Services/WindowsLocalHttpServer.cs (1)
46-50: Improved structured error handling, but consider preserving stack traceThe new implementation properly utilizes the centralized exception handler and provides structured context with StatusCode and RequestUri. However, creating a new HttpRequestException might lose the original stack trace.
Consider preserving the original exception as the inner exception:
- exceptionHandler.Handle(new HttpRequestException(exception.Message), new Dictionary<string, object?>() + exceptionHandler.Handle(new HttpRequestException(exception.Message, exception), new Dictionary<string, object?>()src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Services/ExceptionHandlerBase.cs (2)
16-22: LGTM! Consider adding XML documentation.The parameter rename and early return pattern are good improvements. Consider adding XML documentation to describe the parameters and their purpose.
Add documentation for better maintainability:
+ /// <summary> + /// Handles exceptions with contextual information. + /// </summary> + /// <param name="exception">The exception to handle</param> + /// <param name="parameters">Additional contextual parameters</param> + /// <param name="lineNumber">Auto-populated caller line number</param> + /// <param name="memberName">Auto-populated caller member name</param> + /// <param name="filePath">Auto-populated caller file path</param> public void Handle(Exception exception, Dictionary<string, object?>? parameters = null, [CallerLineNumber] int lineNumber = 0, [CallerMemberName] string memberName = "", [CallerFilePath] string filePath = "")
63-66: Document the rationale for ignored exceptions.The method effectively encapsulates the ignore logic, but it would benefit from documentation explaining why TaskCanceledException is ignored and how derived classes should extend this behavior.
Consider adding documentation and making the method more flexible:
+ /// <summary> + /// Determines whether an exception should be ignored in the logging process. + /// </summary> + /// <param name="exception">The exception to check</param> + /// <returns>True if the exception should be ignored, false otherwise</returns> + /// <remarks> + /// TaskCanceledException is ignored as it typically represents normal cancellation behavior + /// rather than an error condition. + /// </remarks> protected bool IgnoreException(Exception exception) { - return exception is TaskCanceledException; + return exception switch + { + TaskCanceledException => true, + // Add other ignorable exceptions here + _ => false + }; }src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Maui/MauiProgram.cs (1)
Line range hint
1-13: Consider improving the AppCenter secret handling.The current implementation initializes
appCenterSecretto null, making the AppCenter initialization block ineffective. Consider:
- Loading the secret from configuration
- Using a secure secret management solution
Here's a suggested approach:
-string? appCenterSecret = null; +string? appCenterSecret = builder.Configuration.GetValue<string>("AppCenter:Secret");src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/AppInitializer.cs (2)
72-76: Consider enhancing the log message with state details.While the logging structure is good, the message could be more informative about the actual state change.
Consider this enhancement:
- authLogger.LogInformation("Authentication state changed."); + authLogger.LogInformation("Authentication state changed to: {IsAuthenticated}", user.IsAuthenticated());This would provide more context about the nature of the state change while maintaining the telemetry scope.
Line range hint
1-200: Consider breaking down AppInitializer into focused components.The
AppInitializercomponent currently handles multiple concerns including authentication, SignalR, push notifications, culture management, and now enhanced logging. This makes the component harder to maintain and test.Consider:
- Creating separate initialization handlers for each concern:
AuthenticationInitializerSignalRInitializerNotificationInitializerCultureInitializer- Using a facade pattern to coordinate these handlers
- Moving the logging configuration to the respective handlers
This would improve:
- Testability of individual components
- Maintainability through focused responsibilities
- Reusability of initialization logic
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (17)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/AppInitializer.cs(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Services/Contracts/ITelemetryContext.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Services/ExceptionHandlerBase.cs(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Maui/MauiProgram.Services.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Maui/MauiProgram.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Maui/Services/MauiExceptionHandler.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Maui/Services/MauiLocalHttpServer.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Maui/appsettings.json(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Web/Services/WebExceptionHandler.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Windows/MainWindow.xaml.cs(0 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Windows/Program.Services.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Windows/Program.cs(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Windows/Services/WindowsExceptionHandler.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Windows/Services/WindowsLocalHttpServer.cs(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Components/SocialSignedInPage.razor(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/appsettings.Development.json(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/appsettings.json(1 hunks)
💤 Files with no reviewable changes (1)
- src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Windows/MainWindow.xaml.cs
✅ Files skipped from review due to trivial changes (1)
- src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Components/SocialSignedInPage.razor
🧰 Additional context used
🪛 Biome
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/appsettings.Development.json
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/appsettings.json
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 15-15: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 15-15: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
🔇 Additional comments (20)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Web/Services/WebExceptionHandler.cs (1)
7-9: Verify consistent implementation across exception handlers.
The early return pattern looks good. Let's verify this pattern is consistently implemented across other exception handlers.
✅ Verification successful
Early return pattern is consistently implemented across exception handlers
The verification shows that all exception handlers in the codebase follow a consistent pattern of early returns:
- In the Boilerplate project, all handlers (Web, Windows, Maui) use
IgnoreException(exception)check - In the BlazorUI.Demo project, all handlers (Web, Windows, Maui) use
exception is TaskCanceledExceptioncheck
Each implementation properly returns early when the exception should be ignored, followed by calling the base handler for other cases.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if all exception handlers implement the same pattern
# Expected: Similar implementation in MauiExceptionHandler and WindowsExceptionHandler
# Search for all exception handler implementations
ast-grep --pattern 'class $_ : ExceptionHandlerBase
{
$$$
protected override void Handle(Exception exception, Dictionary<string, object> parameters)
{
$$$
}
$$$
}'
Length of output: 320
Script:
#!/bin/bash
# Let's try a different approach using ripgrep to find exception handlers
rg -l "ExceptionHandlerBase" --type cs
# Then examine the content of these files
rg "class.*ExceptionHandlerBase" -A 20 --type cs
Length of output: 15223
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/appsettings.Development.json (1)
11-15: Verify the necessity of dual Application Insights logging configurations.
The configuration now includes both ApplicationInsights and ApplicationInsightsLoggerProvider sections with identical log levels. While this might be intentional, it's worth verifying if both are required.
Consider adding a code comment explaining the purpose of each section if both are indeed necessary.
✅ Verification successful
Let me check the actual usage of these logging configurations to make a more informed decision.
Based on the code inspection, I can now provide a definitive response:
Both Application Insights sections serve distinct purposes and are correctly configured.
The dual configuration is necessary because:
- The
ApplicationInsightssection is used by the core Application Insights SDK when configured viaAddApplicationInsightsTelemetry(configuration)(as seen in both Program.Services.cs files) - The
ApplicationInsightsLoggerProvidersection specifically configures the logging provider aspect, which integrates with .NET's ILogger infrastructure
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usage of both logging providers to verify if they serve different purposes
rg -l "ApplicationInsightsLoggerProvider|ILogger|AddApplicationInsightsTelemetry" --type cs
Length of output: 1712
Script:
#!/bin/bash
# Check Program.Services.cs files for logging configuration
cat "src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Services.cs" "src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Web/Program.Services.cs"
# Check logging builder extensions
cat "src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Extensions/ILoggingBuilderExtensions.cs"
Length of output: 22918
🧰 Tools
🪛 Biome
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Maui/appsettings.json (2)
7-8: LGTM! Logging configuration properly structured.
The addition of AuthenticationManager logging at Information level is well-aligned with the existing logging pattern and will help track authentication state changes effectively.
7-8: Ensure consistent logging configuration across all providers.
While the logging configuration is correct for AppCenterLoggerProvider, the AI summary mentions similar changes in ApplicationInsights sections that are not visible in this file. Consider maintaining consistent logging levels across all providers to ensure uniform visibility of authentication events.
Let's verify the logging configuration in other settings files:
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/appsettings.json (2)
11-13: LGTM! Good addition of AuthenticationManager logging.
The addition of "Information" level logging for the AuthenticationManager is appropriate and consistent with the existing logging pattern used for Entity Framework commands.
14-19: Verify logging configuration consistency.
The configuration is correctly mirrored in both ApplicationInsights and ApplicationInsightsLoggerProvider sections. However, let's verify that all necessary authentication-related components are covered by this logging configuration.
✅ Verification successful
Logging configuration is properly synchronized across environments
The logging configuration for Boilerplate.Client.Core.Services.AuthenticationManager is consistently set to "Information" level across all relevant providers:
- ApplicationInsights and ApplicationInsightsLoggerProvider in Shared/appsettings.json
- AppCenterLoggerProvider in Client/Boilerplate.Client.Maui/appsettings.json
The only authentication-related class found is AuthenticationManager, which is already properly configured in all logging sections.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for other authentication-related components that might need similar logging configuration
# Search for authentication-related services and managers
rg -l "class.*Authentication" --type cs
# Search for any existing logging configurations in other appsettings files
fd appsettings.json --exec grep -l "Authentication.*Information"
Length of output: 387
Script:
#!/bin/bash
# Let's check the content of both appsettings files to compare their logging configurations
echo "=== Shared/appsettings.json ==="
cat src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/appsettings.json
echo -e "\n=== Client/Boilerplate.Client.Maui/appsettings.json ==="
cat src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Maui/appsettings.json
# Let's also check if there are any other authentication-related classes that might need logging
rg "class.*Authentication" -A 5 --type cs
Length of output: 3169
🧰 Tools
🪛 Biome
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 15-15: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 15-15: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Windows/Program.cs (3)
1-2: LGTM: Using directive changes are appropriate.
The addition of the Windows.Services namespace is necessary for the telemetry context implementation.
Line range hint 9-17: Verify AppCenter integration requirements.
The AppCenter configuration is commented out and the secret is null. Please clarify:
- Is AppCenter integration still required?
- If yes, should we properly configure the secret?
- If no, should we remove this commented code?
Let's check for other AppCenter usage:
21-22: Consider thread-safe initialization of telemetry context.
While the initialization sequence is correct, consider these improvements:
- The direct assignment to
ITelemetryContext.Currentmight need thread-safety considerations - Verify that the telemetry context is properly disposed when the application exits
Let's verify the telemetry context usage:
Consider implementing a thread-safe singleton pattern or using dependency injection:
-ITelemetryContext.Current = new WindowsTelemetryContext();
+ITelemetryContext.Current = TelemetryContextFactory.CreateSingleton<WindowsTelemetryContext>();src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Maui/Services/MauiLocalHttpServer.cs (1)
43-48: LGTM! Good job centralizing the error handling.
The change to use exceptionHandler.Handle instead of direct logging improves consistency across the application and aligns with the error handling pattern used in other server implementations.
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Windows/Services/WindowsLocalHttpServer.cs (2)
2-2: LGTM: Required import for HttpRequestException
The addition of System.Net.Http import is necessary for the HttpRequestException usage in the updated error handling.
46-50: Verify consistent exception handling pattern
Let's ensure this exception handling pattern is consistently implemented across other HTTP server implementations.
✅ Verification successful
Let me gather more information about the exception handling implementation in these files to ensure consistency.
Let me check the interface and exception handler implementation to complete the verification.
Let me check the implementation of the exception handler in the Boilerplate project to verify how it handles the context parameters.
Exception handling pattern is consistent and properly implemented
The verification reveals:
- Both Windows and Maui HTTP servers implement identical exception handling patterns
- The exception handler properly accepts and processes the additional context parameters (StatusCode and RequestUri)
- The base implementation (ExceptionHandlerBase) correctly merges these parameters with telemetry context and caller information
- NoopLocalHttpServer is intentionally minimal as it's for platforms without local HTTP server support
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for similar HTTP server implementations
echo "Searching for other HTTP server implementations..."
rg -l "ILocalHttpServer|HandleHttpException" --type cs
# Search for exception handler usage pattern
echo -e "\nChecking exception handler usage patterns..."
rg "exceptionHandler\.Handle.*HttpRequestException" --type cs
Length of output: 1830
Script:
#!/bin/bash
# Check the implementation details of both HTTP servers
echo "=== WindowsLocalHttpServer.cs ==="
rg -A 5 -B 5 "exceptionHandler\.Handle.*HttpRequestException" src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Windows/Services/WindowsLocalHttpServer.cs
echo -e "\n=== MauiLocalHttpServer.cs ==="
rg -A 5 -B 5 "exceptionHandler\.Handle.*HttpRequestException" src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Maui/Services/MauiLocalHttpServer.cs
# Check the NoopLocalHttpServer implementation for comparison
echo -e "\n=== NoopLocalHttpServer.cs ==="
cat src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Services/NoopLocalHttpServer.cs
Length of output: 1870
Script:
#!/bin/bash
# Check the ILocalHttpServer interface
echo "=== ILocalHttpServer.cs ==="
cat src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Services/Contracts/ILocalHttpServer.cs
# Check where the exception handler is defined/used
echo -e "\n=== Searching for exception handler definition ==="
rg "interface.*IExceptionHandler" --type cs
# Check the actual exception handler implementation
echo -e "\n=== Exception Handler Implementation ==="
rg -l "class.*ExceptionHandler.*:.*IExceptionHandler" --type cs
Length of output: 2206
Script:
#!/bin/bash
# Check the ExceptionHandlerBase implementation in Boilerplate
echo "=== ExceptionHandlerBase.cs ==="
cat src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Services/ExceptionHandlerBase.cs
# Check the IExceptionHandler interface in Boilerplate
echo -e "\n=== IExceptionHandler.cs ==="
cat src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Services/Contracts/IExceptionHandler.cs
Length of output: 3179
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Services/ExceptionHandlerBase.cs (1)
25-31: Verify TelemetryContext.ToDictionary implementation.
The parameter handling looks good, but we should verify the TelemetryContext.ToDictionary implementation to ensure it properly handles null values and merges parameters correctly.
✅ Verification successful
Let me try a different search strategy to find the implementation.
Let me try one more search to find the specific ToDictionary implementation in the TelemetryContext.
Parameter handling implementation is correct and safe
The TelemetryContext.ToDictionary implementation is properly handling parameters by:
- Creating a new dictionary that includes any additional parameters passed
- Safely copying all telemetry context properties into the dictionary
- Handling null values implicitly (no explicit null checks needed as nullable types are used)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify TelemetryContext implementation
# Look for the interface and implementation to ensure proper parameter handling
# Search for ITelemetryContext interface
echo "Searching for ITelemetryContext interface definition..."
ast-grep --pattern 'interface ITelemetryContext {
$$$
ToDictionary($$$)
$$$
}'
# Search for TelemetryContext implementation
echo "Searching for TelemetryContext implementation..."
ast-grep --pattern 'class $_ : ITelemetryContext {
$$$
ToDictionary($$$) {
$$$
}
$$$
}'
Length of output: 698
Script:
#!/bin/bash
# Search for TelemetryContext class and interface using ripgrep
echo "Searching for TelemetryContext interface and class..."
rg -A 5 "interface ITelemetryContext|class.*TelemetryContext" --type cs
# Search specifically for ToDictionary method
echo "Searching for ToDictionary method implementation..."
rg -A 10 "ToDictionary.*Dictionary<string, object>" --type cs
Length of output: 12539
Script:
#!/bin/bash
# Search for the specific file content
echo "Searching for ToDictionary implementation in AppTelemetryContext..."
rg -A 15 "ToDictionary.*Dictionary.*" src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Services/AppTelemetryContext.cs
echo "Searching for ToDictionary method in ITelemetryContext..."
rg -A 15 "ToDictionary" src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Services/Contracts/ITelemetryContext.cs
Length of output: 1234
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Windows/Program.Services.cs (2)
31-31: Verify ITelemetryContext.Current initialization.
The registration looks good, but the non-null assertion (!) warrants verification of when and where ITelemetryContext.Current is initialized to ensure it's never null when this service registration occurs.
Let's verify the initialization:
✅ Verification successful
ITelemetryContext initialization is properly ordered and safe
The initialization of ITelemetryContext.Current occurs early in the Main method, before any service registration or application startup. This ensures the non-null assertion in Program.Services.cs is safe to use.
- ✓ Initialized at line 22 in Program.cs
- ✓ Occurs before VelopackApp.Build() and application startup
- ✓ No conditional blocks that could skip the initialization
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find where ITelemetryContext.Current is initialized
# Expected: Find initialization before this service registration
# Search for ITelemetryContext.Current assignments
rg -l "ITelemetryContext\.Current\s*="
# Search for WindowsTelemetryContext instantiation
ast-grep --pattern 'new WindowsTelemetryContext'
Length of output: 576
Script:
#!/bin/bash
# Let's check the initialization in Program.cs to verify the order
cat "src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Windows/Program.cs"
Length of output: 2036
Line range hint 42-67: Verify telemetry data flow with the new ITelemetryContext registration.
The logging configuration looks comprehensive. Since we've changed how ITelemetryContext is registered, let's ensure that telemetry data flows correctly through all configured providers, especially AppCenter and ApplicationInsights where scopes are enabled.
Let's check the telemetry initialization and usage:
✅ Verification successful
Based on the search results, I can see that:
WindowsTelemetryInitializeris properly implemented and usesITelemetryContextto initialize telemetry data- The telemetry context is registered correctly in the DI container:
services.AddSingleton(ITelemetryContext.Current!);
- The telemetry context is set at startup:
ITelemetryContext.Current = new WindowsTelemetryContext();
- The telemetry initialization is properly configured in the logging setup:
config.TelemetryInitializers.Add(new WindowsTelemetryInitializer());
No issues found with telemetry data flow after ITelemetryContext changes.
The telemetry context is properly registered and initialized, and the telemetry initializer correctly maps the context data to Application Insights telemetry. The changes to ITelemetryContext registration do not impact the telemetry data flow.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify telemetry initialization and usage
# Expected: Find how telemetry context data flows through the system
# Search for WindowsTelemetryInitializer implementation
ast-grep --pattern 'class WindowsTelemetryInitializer'
# Search for usage of ITelemetryContext in logging
rg "ITelemetryContext" -A 5
Length of output: 213612
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Maui/MauiProgram.Services.cs (3)
Line range hint 42-86: Comprehensive logging configuration looks good!
The logging setup demonstrates good practices:
- Environment-specific providers (Debug, Browser console for dev)
- Platform-specific handling (Event log for Windows)
- Proper scope configuration for AppCenter and AppInsights
- Clear separation of concerns
Line range hint 7-24: Well-structured service configuration!
The service registration demonstrates solid architectural decisions:
- Appropriate service lifetimes
- Proper configuration validation
- Clean platform-specific isolation
Also applies to: 87-102
32-32:
Verify ITelemetryContext initialization and add null safety.
The registration of ITelemetryContext.Current! raises concerns:
- The null-forgiving operator doesn't prevent runtime null references
- It's unclear where/when
Currentis initialized
Consider adding null safety and documentation:
- services.AddSingleton(ITelemetryContext.Current!);
+ // Initialize telemetry context if not already set
+ services.AddSingleton<ITelemetryContext>(sp =>
+ ITelemetryContext.Current ?? throw new InvalidOperationException(
+ "ITelemetryContext.Current must be initialized before service registration"));✅ Verification successful
The null-forgiving operator is safe to use here.
The telemetry context is properly initialized before service registration:
- In
MauiProgram.cs:ITelemetryContext.Current = new MauiTelemetryContext();is called at the start - The service registration happens after initialization in the same startup flow
- Similar pattern is followed in Windows client:
ITelemetryContext.Current = new WindowsTelemetryContext();
The null-forgiving operator is appropriate here as the code guarantees initialization. The original review's suggestion would add unnecessary runtime checks.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find where ITelemetryContext.Current is initialized
ast-grep --pattern 'ITelemetryContext.Current = $_'
# Find usages of ITelemetryContext
rg -A 3 'ITelemetryContext'
Length of output: 16156
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Maui/MauiProgram.cs (1)
Line range hint 40-209: Well-structured platform-specific code.
The code demonstrates good practices in handling platform-specific requirements:
- Clear separation using conditional compilation
- Proper error handling for each platform
- Well-organized lifecycle events
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/AppInitializer.cs (1)
25-25: LGTM: Well-typed logger for authentication events.
The addition of a dedicated logger for authentication events follows good practices by:
- Using proper generic typing for filtering capabilities
- Following the existing dependency injection pattern
- Maintaining consistent initialization style
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Maui/MauiProgram.cs
Show resolved
Hide resolved
ysmoradi
deleted the
9098-boilerplate-project-templates-logging-has-small-issues
branch
This closes #9098
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Chores