Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(hr): Improve retry logic when using UpdateFile in case of NoChanges #19005

Merged
merged 1 commit into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,10 @@
/// </summary>
private class HotReloadServerOperation
{
public readonly static int DefaultAutoRetryIfNoChangesAttempts = 3;

Check notice on line 209 in src/Uno.UI.RemoteControl.Server.Processors/HotReload/ServerHotReloadProcessor.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/Uno.UI.RemoteControl.Server.Processors/HotReload/ServerHotReloadProcessor.cs#L209

Replace this 'static readonly' declaration with 'const'.

public readonly static TimeSpan DefaultAutoRetryIfNoChangesDelay = TimeSpan.FromMilliseconds(500);

// Delay to wait without any update to consider operation was aborted.
private static readonly TimeSpan _timeoutDelay = TimeSpan.FromSeconds(30);

Expand All @@ -219,7 +223,11 @@
private ImmutableHashSet<string> _filePaths;
private int /* HotReloadResult */ _result = -1;
private CancellationTokenSource? _deferredCompletion;

// In VS we forcefully request to VS to hot-reload application, but in some cases the changes are not detected by VS and it returns a NoChanges result.
// In such cases we can retry the hot-reload request to VS to let it process the file updates.
private int _noChangesRetry;
private TimeSpan _noChangesRetryDelay = DefaultAutoRetryIfNoChangesDelay;

public long Id { get; } = Interlocked.Increment(ref _count);

Expand Down Expand Up @@ -294,8 +302,11 @@
/// <summary>
/// Configure a simple auto-retry strategy if no changes are detected.
/// </summary>
public void EnableAutoRetryIfNoChanges()
=> _noChangesRetry = 1;
public void EnableAutoRetryIfNoChanges(int? attempts, TimeSpan? delay)
{
_noChangesRetry = attempts ?? DefaultAutoRetryIfNoChangesAttempts;
_noChangesRetryDelay = delay ?? DefaultAutoRetryIfNoChangesDelay;
}

/// <summary>
/// As errors might get a bit after the complete from the IDE, we can defer the completion of the operation.
Expand All @@ -322,10 +333,17 @@
{
if (_result is -1
&& result is HotReloadServerResult.NoChanges
&& Interlocked.Decrement(ref _noChangesRetry) >= 0
&& await _owner.RequestHotReloadToIde(Id))
&& Interlocked.Decrement(ref _noChangesRetry) >= 0)
{
return;
if (_noChangesRetryDelay is { TotalMilliseconds: > 0 })
{
await Task.Delay(_noChangesRetryDelay);
}

if (await _owner.RequestHotReloadToIde(Id))
{
return;
}
}

Debug.Assert(result != HotReloadServerResult.InternalError || exception is not null); // For internal error we should always provide an exception!
Expand Down Expand Up @@ -463,7 +481,7 @@
};
if ((int)result < 300 && !message.IsForceHotReloadDisabled)
{
hotReload.EnableAutoRetryIfNoChanges();
hotReload.EnableAutoRetryIfNoChanges(message.ForceHotReloadAttempts, message.ForceHotReloadDelay);
await RequestHotReloadToIde(hotReload.Id);
}

Expand Down Expand Up @@ -579,7 +597,11 @@
{
if (e.FullPath.Equals(file.FullName, StringComparison.OrdinalIgnoreCase))
{
await Task.Delay(500);
if ((message.ForceHotReloadDelay ?? HotReloadServerOperation.DefaultAutoRetryIfNoChangesDelay) is { TotalMilliseconds: > 0 } delay)
{
await Task.Delay(delay);
}

tcs.TrySetResult();
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,16 @@ public record struct UpdateRequest(
/// <remarks>This includes the time to apply the delta locally and then to run all local handlers.</remarks>
public TimeSpan LocalHotReloadTimeout { get; set; } = TimeSpan.FromSeconds(5);

/// <summary>
/// When <see cref="WaitForHotReload"/> the delay to wait before retrying a hot-reload in Visual Studio if no changes are detected.
/// </summary>
public TimeSpan? HotReloadNoChangesRetryDelay { get; set; }

/// <summary>
/// When <see cref="WaitForHotReload"/> the number of times to retry the hot reload in Visual Studio if no changes are detected.
/// </summary>
public int? HotReloadNoChangesRetryAttempts { get; set; }

public UpdateRequest WithExtendedTimeouts(float? factor = null)
{
factor ??= Debugger.IsAttached ? 10 : 30;
Expand Down Expand Up @@ -116,7 +126,14 @@ public async Task<UpdateResult> TryUpdateFileAsync(UpdateRequest req, Cancellati
// As the local HR is not really ID trackable (trigger by VS without any ID), we capture the current ID here to make sure that if HR completes locally before we get info from the server, we won't miss it.
var currentLocalHrId = GetCurrentLocalHotReloadId();

var request = new UpdateFile { FilePath = req.FilePath, OldText = req.OldText, NewText = req.NewText };
var request = new UpdateFile
{
FilePath = req.FilePath,
OldText = req.OldText,
NewText = req.NewText,
ForceHotReloadDelay = req.HotReloadNoChangesRetryDelay,
ForceHotReloadAttempts = req.HotReloadNoChangesRetryAttempts
};
var response = await UpdateFileCoreAsync(request, req.ServerUpdateTimeout, ct);

if (response.Result is FileUpdateResult.NoChanges)
Expand Down
12 changes: 12 additions & 0 deletions src/Uno.UI.RemoteControl/HotReload/Messages/UpdateFile.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,18 @@ public class UpdateFile : IMessage
[JsonProperty]
public bool IsForceHotReloadDisabled { get; set; }

/// <summary>
/// The delay to wait before forcing (**OR RETRYING**) a hot reload in Visual Studio.
/// </summary>
[JsonProperty]
public TimeSpan? ForceHotReloadDelay { get; set; }

/// <summary>
/// Number of times to retry the hot reload in Visual Studio **if not changes are detected**.
/// </summary>
[JsonProperty]
public int? ForceHotReloadAttempts { get; set; }

[JsonIgnore]
public string Scope => WellKnownScopes.HotReload;

Expand Down
Loading