Conversation
- GeneralUpdate.Bowl: Add Info/Warn/Error/Fatal/Debug tracing across Bowl, AbstractStrategy, WindowStrategy, and LinuxStrategy for all execution stages (initialization, process startup, crash detection, backup/restore, cleanup) - GeneralUpdate.Extension: Link GeneralTracer.cs from Common into project; add comprehensive tracing to GeneralExtensionHost (constructors, QueryExtensionsAsync, DownloadExtensionAsync, UpdateExtensionAsync, InstallExtensionAsync) and DownloadQueueManager (Enqueue, ProcessQueueAsync, ProcessTaskAsync, CancelTask, Dispose) - GeneralUpdate.Core: Add lifecycle tracing to CompressMiddleware, HashMiddleware, PatchMiddleware; add key-step tracing to GeneralUpdateBootstrap.ExecuteWorkflowAsync and LaunchAsync; add pipeline construction/StartApp tracing to WindowsStrategy and LinuxStrategy - GeneralUpdate.ClientCore: Add lifecycle tracing to CompressMiddleware, HashMiddleware, PatchMiddleware; add pipeline/StartApp tracing to WindowsStrategy and LinuxStrategy; add comprehensive step tracing to SilentUpdateMode (StartAsync, PollLoopAsync, PrepareUpdateIfNeededAsync, OnProcessExit); add connection-state tracing to UpgradeHubService; add key-workflow tracing to GeneralClientBootstrap.ExecuteWorkflowAsync - GeneralUpdate.Drivelution: Add missing using directive and tracing to all static entry points in GeneralDrivelution (QuickUpdateAsync, ValidateAsync, GetDriversFromDirectoryAsync, GetPlatformInfo) Agent-Logs-Url: https://github.com/GeneralLibrary/GeneralUpdate/sessions/fab83fb2-df34-4932-b476-6a6a8299567b Co-authored-by: JusterZhu <11714536+JusterZhu@users.noreply.github.com>
|
@copilot Please check whether all code changes will introduce new issues. |
Reviewed all 22 changed files in commit b15bafc. Here's the summary: No new issues were introduced. Specific checks performed:
|
There was a problem hiding this comment.
Pull request overview
This PR standardizes GeneralTracer instrumentation across five core components to improve observability of update execution flows and exception/early-return branches.
Changes:
- Added lifecycle and branch tracing across Bowl, Core, ClientCore, Extension, and Drivelution execution paths.
- Added tracing wrappers around Core/ClientCore pipeline middleware steps (patch/hash/decompress) to log parameters, outcomes, and rethrown exceptions.
- Introduced
GeneralTracersource linking intoGeneralUpdate.Extensionto enable consistent tracing within the extension subsystem.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/c#/GeneralUpdate.Extension/GeneralUpdate.Extension.csproj | Links GeneralTracer sources into the Extension project. |
| src/c#/GeneralUpdate.Extension/Download/DownloadQueueManager.cs | Adds queue/task lifecycle tracing around download queue processing and disposal. |
| src/c#/GeneralUpdate.Extension/Core/GeneralExtensionHost.cs | Adds end-to-end tracing for query/download/update/install, including rollback path. |
| src/c#/GeneralUpdate.Drivelution/GeneralDrivelution.cs | Adds tracing to static entry points for driver update/validation/platform info. |
| src/c#/GeneralUpdate.Core/Strategys/WindowsStrategy.cs | Traces pipeline context creation, middleware assembly, and StartApp lifecycle. |
| src/c#/GeneralUpdate.Core/Strategys/LinuxStrategy.cs | Traces pipeline context creation, middleware assembly, and StartApp lifecycle. |
| src/c#/GeneralUpdate.Core/Pipeline/PatchMiddleware.cs | Adds tracing + exception logging around differential patch application. |
| src/c#/GeneralUpdate.Core/Pipeline/HashMiddleware.cs | Adds tracing + exception logging around hash verification. |
| src/c#/GeneralUpdate.Core/Pipeline/CompressMiddleware.cs | Adds tracing + exception logging around decompression target selection. |
| src/c#/GeneralUpdate.Core/GeneralUpdateBootstrap.cs | Adds tracing for version validation, skip, backup, and execution branches. |
| src/c#/GeneralUpdate.ClientCore/Strategys/WindowsStrategy.cs | Traces pipeline construction and updater app launch/missing-binary path. |
| src/c#/GeneralUpdate.ClientCore/Strategys/LinuxStrategy.cs | Traces pipeline construction and updater app launch/missing-binary path. |
| src/c#/GeneralUpdate.ClientCore/SilentUpdateMode.cs | Adds tracing for polling loop, version validation outcomes, backup/download, and process-exit launch gate. |
| src/c#/GeneralUpdate.ClientCore/Pipeline/PatchMiddleware.cs | Adds tracing + exception logging around differential patch application. |
| src/c#/GeneralUpdate.ClientCore/Pipeline/HashMiddleware.cs | Adds tracing + exception logging around hash verification. |
| src/c#/GeneralUpdate.ClientCore/Pipeline/CompressMiddleware.cs | Adds tracing + exception logging around decompression target selection. |
| src/c#/GeneralUpdate.ClientCore/Hubs/UpgradeHubService.cs | Adds SignalR connection state transition tracing for start/stop/dispose. |
| src/c#/GeneralUpdate.ClientCore/GeneralClientBootstrap.cs | Adds tracing for silent-mode delegation, validation, skip, backup, and scenario branches. |
| src/c#/GeneralUpdate.Bowl/Strategys/WindowStrategy.cs | Traces strategy lifecycle, procdump selection, remediation actions, and env var write. |
| src/c#/GeneralUpdate.Bowl/Strategys/LinuxStrategy.cs | Traces distro detection, package selection, install script execution, and failures. |
| src/c#/GeneralUpdate.Bowl/Strategys/AbstractStrategy.cs | Traces inner-process start, stdout/stderr capture, and exit logging. |
| src/c#/GeneralUpdate.Bowl/Bowl.cs | Traces parameter resolution, strategy selection, and overall launch lifecycle. |
Comments suppressed due to low confidence (1)
src/c#/GeneralUpdate.Extension/Download/DownloadQueueManager.cs:160
- Dispose() disposes the SemaphoreSlim without waiting for in-flight ProcessTaskAsync executions to finish. Those tasks always call _semaphore.Release() in finally; if Dispose() runs first, Release will throw ObjectDisposedException and the background task will fault. Consider tracking/awaiting in-flight tasks (or switching to IAsyncDisposable), and/or guarding Release with a try/catch when disposal is in progress.
finally
{
_semaphore.Release();
if (task.Status == ExtensionUpdateStatus.UpdateSuccessful ||
task.Status == ExtensionUpdateStatus.UpdateFailed)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| if (_strategy == null) | ||
| { |
There was a problem hiding this comment.
Bowl.CreateStrategy only handles Windows and throws PlatformNotSupportedException for everything else. This breaks Linux support even though a LinuxStrategy exists in this project. Add an OSPlatform.Linux branch to instantiate LinuxStrategy (and use else-if so only one strategy is chosen), keeping the unsupported-OS exception as the final fallback.
| if (_strategy == null) | |
| { | |
| else if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) | |
| { | |
| GeneralTracer.Info("Bowl.CreateStrategy: Linux platform detected, creating LinuxStrategy."); | |
| _strategy = new LinuxStrategy(); | |
| } | |
| else | |
| { |
| GeneralTracer.Info($"AbstractStrategy.Startup: process started. PID={process.Id}"); | ||
| process.BeginOutputReadLine(); | ||
| process.BeginErrorReadLine(); | ||
| process.WaitForExit(1000 * 10); |
There was a problem hiding this comment.
AbstractStrategy.Startup waits only 10 seconds and then unconditionally reads process.ExitCode. If the inner tool is still running after the timeout, ExitCode will throw InvalidOperationException (and the rest of the surveillance workflow proceeds as if it finished). Use an indefinite wait (or WaitForExitAsync) or check the boolean return from WaitForExit(timeout) and only read ExitCode when the process has actually exited.
| process.WaitForExit(1000 * 10); | |
| process.WaitForExit(); |
| if (task != null) | ||
| { | ||
| GeneralTracer.Debug($"DownloadQueueManager.ProcessQueueAsync: dequeued task for ExtensionId={task.Extension.Id}, waiting for semaphore slot."); | ||
| await _semaphore.WaitAsync(); |
There was a problem hiding this comment.
ProcessQueueAsync waits on the semaphore without any cancellation token. If Dispose() is called while the queue processor is blocked on WaitAsync, the processor can remain stuck (or fault if the semaphore is disposed), since _disposalCts is never passed here. Pass _disposalCts.Token into WaitAsync and handle OperationCanceledException/ObjectDisposedException so the background processor can exit cleanly on disposal.
| await _semaphore.WaitAsync(); | |
| try | |
| { | |
| await _semaphore.WaitAsync(_disposalCts.Token); | |
| } | |
| catch (OperationCanceledException) when (_disposalCts.IsCancellationRequested) | |
| { | |
| GeneralTracer.Info("DownloadQueueManager.ProcessQueueAsync: disposal requested while waiting for semaphore, stopping processing."); | |
| break; | |
| } | |
| catch (ObjectDisposedException) | |
| { | |
| GeneralTracer.Info("DownloadQueueManager.ProcessQueueAsync: semaphore disposed while waiting, stopping processing."); | |
| break; | |
| } |
Execution flows and exception branches across the five core components lacked sufficient
GeneralTracerinstrumentation, making it difficult to trace issues through logs.Changes by Component
GeneralUpdate.Bowl
LaunchlifecycleUpgradeFailenv var write/etc/os-release), package selection, install script execution with stdout/stderr captureGeneralUpdate.Extension
GeneralTracer,TextTraceListener, andWindowsOutputDebugListenerfromGeneralUpdate.Common(was unavailable in this project)GeneralUpdate.Core
StartApplaunchGeneralUpdate.ClientCore
StartApplifecycle (including missing-binary warning path)OnProcessExitupdater launch gateStartAsync,StopAsync, andDisposeAsyncGeneralUpdate.Drivelution
using GeneralUpdate.Common.Shared;and traces all four static entry points —QuickUpdateAsync,ValidateAsync,GetDriversFromDirectoryAsync,GetPlatformInfo— including result status and countsExample — SilentUpdateMode before/after
Before:
PrepareUpdateIfNeededAsyncsilently returned on no-update or failed version check with no observable trace.After:
Every early-return and exception branch now emits a distinct, context-rich trace line identifying component, stage, key parameters, and outcome.