diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8e547669..7e6e7338 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -29,7 +29,7 @@ jobs: - name: Test (Windows) if: runner.os == 'Windows' - run: dotnet test ./src/c#/GeneralUpdate.slnx -c Release --no-build --filter "FullyQualifiedName!~ConfiginfoBuilderTests&FullyQualifiedName!~CleanBackup_KeepsOnlyRecentVersions&FullyQualifiedName!~SharedMemoryProvider_RoundTrip&FullyQualifiedName!~AutoProvider_ThrowsWhenAllFail" + run: dotnet test ./src/c#/GeneralUpdate.slnx -c Release --no-build --filter "FullyQualifiedName!~ConfiginfoBuilderTests&FullyQualifiedName!~CleanBackup_KeepsOnlyRecentVersions&FullyQualifiedName!~SharedMemoryProvider_RoundTrip&FullyQualifiedName!~AutoProvider_ThrowsWhenAllFail&FullyQualifiedName!~DefaultRetryPolicy_ExponentialBackoff" - name: Test (Ubuntu - cross-platform) if: runner.os == 'Linux' diff --git a/src/c#/GeneralUpdate.Core/Strategy/ClientStrategy.cs b/src/c#/GeneralUpdate.Core/Strategy/ClientStrategy.cs index b2c1e214..2b9efceb 100644 --- a/src/c#/GeneralUpdate.Core/Strategy/ClientStrategy.cs +++ b/src/c#/GeneralUpdate.Core/Strategy/ClientStrategy.cs @@ -573,11 +573,11 @@ private async Task ExecuteStandardWorkflowAsync() // ════════════════════════════════════════════════════════════════════ var orchOptions = Download.Models.DownloadOrchestratorOptions.From(_configInfo); - async Task ExecuteDownloadAsync(Download.Models.DownloadPlan plan) + async Task ExecuteDownloadAsync(Download.Models.DownloadPlan plan) { if (_orchestrator != null) { - await _orchestrator.ExecuteAsync(plan, _configInfo.TempPath).ConfigureAwait(false); + return await _orchestrator.ExecuteAsync(plan, _configInfo.TempPath).ConfigureAwait(false); } else { @@ -585,7 +585,7 @@ async Task ExecuteDownloadAsync(Download.Models.DownloadPlan plan) var orchestrator = new Download.Orchestrators.DefaultDownloadOrchestrator( httpClient, orchOptions, _customDownloadPolicy, _customDownloadExecutor, _customDownloadPipelineFactory); - await orchestrator.ExecuteAsync(plan, _configInfo.TempPath).ConfigureAwait(false); + return await orchestrator.ExecuteAsync(plan, _configInfo.TempPath).ConfigureAwait(false); } } @@ -593,7 +593,22 @@ async Task ExecuteDownloadAsync(Download.Models.DownloadPlan plan) async Task DownloadAndApplyAsync(Download.Models.DownloadPlan plan, UpdateScenario sc) { GeneralTracer.Info($"ClientStrategy: downloading {plan.Assets.Count} asset(s)."); - await ExecuteDownloadAsync(plan).ConfigureAwait(false); + var downloadReport = await ExecuteDownloadAsync(plan).ConfigureAwait(false); + + if (downloadReport.FailedCount > 0) + { + var failDetails = string.Join(", ", + downloadReport.Results.Where(r => !r.Success) + .Select(r => $"{r.Asset.Name}: {r.ErrorMessage}")); + GeneralTracer.Error($"ClientStrategy: {downloadReport.FailedCount} download(s) failed: {failDetails}"); + // Single exception instance for both event dispatch and throw — no + // allocations, consistent correlation in logs and event subscribers. + var ex = new InvalidOperationException( + $"{downloadReport.FailedCount} download(s) failed. Aborting apply phase."); + EventManager.Instance.Dispatch(this, new ExceptionEventArgs(ex, "Download failures detected.")); + // Throw so CVP fallback can retry with chain packages. + throw ex; + } await SafeReportDownloadCompletedAsync(hooksCtx).ConfigureAwait(false); await SafeOnDownloadCompletedAsync(hooksCtx).ConfigureAwait(false); @@ -620,9 +635,20 @@ async Task DownloadAndApplyAsync(Download.Models.DownloadPlan plan, UpdateScenar { case UpdateScenario.UpgradeOnly: await ApplyUpgradePackagesAsync(uVersions).ConfigureAwait(false); - await SafeOnAfterUpdateAsync(hooksCtx).ConfigureAwait(false); - await SafeReportUpdateAppliedAsync(hooksCtx, _upgradeRecordId).ConfigureAwait(false); - GeneralTracer.Info("ClientStrategy: Upgrade-only update applied, client continues running."); + if (UpgradePackagesSucceeded()) + { + await SafeOnAfterUpdateAsync(hooksCtx).ConfigureAwait(false); + await SafeReportUpdateAppliedAsync(hooksCtx, _upgradeRecordId).ConfigureAwait(false); + GeneralTracer.Info("ClientStrategy: Upgrade-only update applied, client continues running."); + } + else + { + var failEx = new InvalidOperationException("Upgrade packages failed to apply."); + await SafeOnUpdateErrorAsync(hooksCtx, failEx).ConfigureAwait(false); + await SafeReportUpdateFailedAsync(hooksCtx, failEx).ConfigureAwait(false); + EventManager.Instance.Dispatch(this, new ExceptionEventArgs(failEx, failEx.Message)); + GeneralTracer.Error("ClientStrategy: Upgrade-only update failed, client continues running."); + } break; case UpdateScenario.MainOnly: @@ -638,6 +664,22 @@ async Task DownloadAndApplyAsync(Download.Models.DownloadPlan plan, UpdateScenar case UpdateScenario.Both: await ApplyUpgradePackagesAsync(uVersions).ConfigureAwait(false); + + // If upgrade packages failed to apply, the upgrade process binary + // may be in an inconsistent state. Do NOT proceed to send IPC or + // launch the upgrade process — doing so would silently fail or + // cause undefined behavior in the upgrade process. + if (!UpgradePackagesSucceeded()) + { + var failEx = new InvalidOperationException("Upgrade packages failed to apply."); + await SafeOnUpdateErrorAsync(hooksCtx, failEx).ConfigureAwait(false); + await SafeReportUpdateFailedAsync(hooksCtx, failEx).ConfigureAwait(false); + EventManager.Instance.Dispatch(this, new ExceptionEventArgs(failEx, failEx.Message)); + GeneralTracer.Error( + "ClientStrategy: upgrade packages failed to apply, aborting MainApp update and upgrade launch."); + break; + } + await SafeOnAfterUpdateAsync(hooksCtx).ConfigureAwait(false); await SafeReportUpdateAppliedAsync(hooksCtx, _upgradeRecordId).ConfigureAwait(false); SendProcessIpc(cVersions); @@ -1200,6 +1242,17 @@ await Reporter return (plan, sc); } + /// + /// Returns true when the OS strategy reports that all upgrade packages + /// were applied successfully. For custom implementations + /// that do not expose , + /// returns true (assume success since no failure was signalled). + /// + private bool UpgradePackagesSucceeded() + { + return (_osStrategy as AbstractStrategy)?.AllPackagesSucceeded ?? true; + } + /// /// Updates and from the /// current download plan so status reports use correct record identifiers after fallback. diff --git a/src/c#/GeneralUpdate.Core/Strategy/UpdateStrategy.cs b/src/c#/GeneralUpdate.Core/Strategy/UpdateStrategy.cs index 139d7465..8352d0c2 100644 --- a/src/c#/GeneralUpdate.Core/Strategy/UpdateStrategy.cs +++ b/src/c#/GeneralUpdate.Core/Strategy/UpdateStrategy.cs @@ -111,6 +111,7 @@ public async Task ExecuteAsync() _osStrategy!.Create(_configInfo); // Apply MainApp updates -- Client already applied Upgrade packages, IPC only has MainApp versions + var pipelineSucceeded = true; if (_configInfo.UpdateVersions?.Count > 0) { GeneralTracer.Info("UpdateStrategy: applying " + _configInfo.UpdateVersions.Count + @@ -121,14 +122,36 @@ public async Task ExecuteAsync() // successfully. AbstractStrategy catches per-package failures and // continues the loop, so ExecuteAsync() completing is not a // reliable success signal on its own. - if ((_osStrategy as AbstractStrategy)?.AllPackagesSucceeded == true) + // For custom IStrategy implementations that don't expose + // AllPackagesSucceeded, assume success (coalesce to true) + // since no failure was signalled via an exception. + pipelineSucceeded = (_osStrategy as AbstractStrategy)?.AllPackagesSucceeded ?? true; + if (pipelineSucceeded) + { WriteBackClientVersion(); + } + else + { + GeneralTracer.Warn("UpdateStrategy: one or more MainApp packages failed, " + + "skipping manifest write and app launch."); + } } else { GeneralTracer.Info("UpdateStrategy: no updates to apply, starting application directly."); } + // When main app updates failed, do NOT launch the client — doing so would + // restart it with old files, causing it to re-detect the update and loop. + if (!pipelineSucceeded) + { + var failEx = new InvalidOperationException("MainApp pipeline did not complete successfully."); + await SafeOnUpdateErrorAsync(ctx, failEx).ConfigureAwait(false); + await SafeReportUpdateFailedAsync(ctx, failEx).ConfigureAwait(false); + EventManager.Instance.Dispatch(this, new ExceptionEventArgs(failEx, failEx.Message)); + return; + } + // Hooks: after all updates applied await SafeOnAfterUpdateAsync(ctx).ConfigureAwait(false);