Skip to content

Commit 4cf0b9b

Browse files
committed
Let interop calls cut off by disposal finish without throwing
A command or query still in flight when the component was disposed threw once the JS side went away. In a reloaded BlazorWebView the page no longer holds the instance, so the call fails with a JSException; elsewhere it is JSDisconnectedException, ObjectDisposedException or TaskCanceledException. Calls made after disposal already did nothing, but these were issued before it. A caller that fired one off without awaiting - the usual way to scroll a list back to the top after a filter change - saw it only as an unobserved task exception, so apps wrapped the calls in catches of their own: SatisFIT's exercise picker catches ObjectDisposedException around ScrollToTop, which misses the other three. InvokeInstance and QueryInstance now catch a failure that arrives after disposal started: a command completes, a query returns its default, and the failure is logged as teardown already does - Debug for the known ways the JS side goes away, Warning for anything else. A failure while the component is alive still throws. The render path had the same gap: a render's setOptions, setItems or measureRenderedRows in flight at disposal stopped the render when it succeeded, but escaped as an unhandled render exception when it failed. Those calls now go through InvokeInstance too. Tests: ScrollToTop in flight at disposal, for each of the four exceptions and an unexpected one (logged at Debug, Warning for the last); GetSnapshot in flight returns null; the render-race theory now also fails each held call. All nine fail without the catches. A failure while alive still throws.
1 parent 4177bf1 commit 4cf0b9b

3 files changed

Lines changed: 121 additions & 10 deletions

File tree

‎README.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,8 @@ var snapshot = await _virtualize.GetSnapshot();
129129
var measurements = await _virtualize.TakeSnapshot();
130130
```
131131

132+
These are safe to fire off without awaiting while the component may be going away. Called after disposal they do nothing, and one still in flight when the component is disposed completes without throwing: a query returns its default, and the failure is logged - at Debug when the JS side simply went away.
133+
132134
### Two-phase rendering
133135

134136
### Directional overscan

‎src/Blazor.TanStack.VirtualCore/AdaptiveVirtualize.razor.cs‎

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ protected override async Task OnAfterRenderAsync(bool firstRender)
290290
if (!ReferenceEquals(_lastOptionsRef, Options))
291291
{
292292
_lastOptionsRef = Options;
293-
await _jsInstance.InvokeVoidAsync("setOptions", Options);
293+
await InvokeInstance("setOptions", Options);
294294

295295
// The component can be disposed during any await here, and disposal releases the
296296
// instance. Every call this render made after that point would fail on the render path.
@@ -310,7 +310,7 @@ protected override async Task OnAfterRenderAsync(bool firstRender)
310310
// setItems.
311311
_lastItemsRef = Items;
312312
BuildDescriptors();
313-
await _jsInstance.InvokeVoidAsync("setItems", _itemKeys, _itemSizes);
313+
await InvokeInstance("setItems", _itemKeys, _itemSizes);
314314

315315
if (_isDisposed)
316316
{
@@ -346,7 +346,7 @@ protected override async Task OnAfterRenderAsync(bool firstRender)
346346
_lastMeasuredFirstIndex = firstIndex;
347347
_lastMeasuredLastIndex = lastIndex;
348348

349-
await _jsInstance.InvokeVoidAsync("measureRenderedRows");
349+
await InvokeInstance("measureRenderedRows");
350350

351351
if (_isDisposed)
352352
{
@@ -479,24 +479,47 @@ public async Task<VirtualItem[]> TakeSnapshot()
479479
return await QueryInstance<VirtualItem[]>("takeSnapshot") ?? [];
480480
}
481481

482+
/// <summary>Calls a method on the JS instance; a call made after disposal does nothing.</summary>
483+
/// <remarks>
484+
/// A call still in flight when the component is disposed can fail once the JS side goes away -
485+
/// in a reloaded BlazorWebView the page no longer holds the instance - and is then logged rather
486+
/// than thrown: the list it was meant for is gone, and a caller that fired it off without
487+
/// awaiting would see it only as an unobserved task exception, a render as an unhandled one.
488+
/// </remarks>
482489
private async Task InvokeInstance(string method, params object?[] args)
483490
{
484491
if (_jsInstance is null)
485492
{
486493
return;
487494
}
488495

489-
await _jsInstance.InvokeVoidAsync(method, args);
496+
try
497+
{
498+
await _jsInstance.InvokeVoidAsync(method, args);
499+
}
500+
catch (Exception exception) when (_isDisposed)
501+
{
502+
LogTeardownFailure(exception, $"calling {method}");
503+
}
490504
}
491505

506+
/// <summary>Like <see cref="InvokeInstance"/>; a query cut off by disposal returns the default.</summary>
492507
private async Task<TResult?> QueryInstance<TResult>(string method, params object?[] args)
493508
{
494509
if (_jsInstance is null)
495510
{
496511
return default;
497512
}
498513

499-
return await _jsInstance.InvokeAsync<TResult>(method, args);
514+
try
515+
{
516+
return await _jsInstance.InvokeAsync<TResult>(method, args);
517+
}
518+
catch (Exception exception) when (_isDisposed)
519+
{
520+
LogTeardownFailure(exception, $"calling {method}");
521+
return default;
522+
}
500523
}
501524

502525
/// <summary>

‎tests/Blazor.TanStack.VirtualCore.Tests/AdaptiveVirtualizeTests.cs‎

Lines changed: 91 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -193,10 +193,13 @@ public async Task DisposeAsync_JSObjectAlreadyGoneFromThePage_DoesNotThrow()
193193
}
194194

195195
[Theory]
196-
[InlineData("setOptions")]
197-
[InlineData("setItems")]
198-
[InlineData("measureRenderedRows")]
199-
public async Task DisposeAsync_WhileARenderAwaitsTheJSInstance_ThatRenderGoesNoFurther(string callInFlight)
196+
[InlineData("setOptions", false)]
197+
[InlineData("setItems", false)]
198+
[InlineData("measureRenderedRows", false)]
199+
[InlineData("setOptions", true)]
200+
[InlineData("setItems", true)]
201+
[InlineData("measureRenderedRows", true)]
202+
public async Task DisposeAsync_WhileARenderAwaitsTheJSInstance_ThatRenderGoesNoFurther(string callInFlight, bool callFails)
200203
{
201204
// Arrange
202205
using var context = CreateContext();
@@ -221,7 +224,15 @@ public async Task DisposeAsync_WhileARenderAwaitsTheJSInstance_ThatRenderGoesNoF
221224

222225
// Act
223226
var disposal = component.InvokeAsync(async () => await component.Instance.DisposeAsync());
224-
heldCall.SetVoidResult();
227+
// In a reloaded BlazorWebView the call fails: the page it reaches no longer holds the instance.
228+
if (callFails)
229+
{
230+
heldCall.SetException(new JSException("JS object instance with ID 20 does not exist (has it been disposed?)"));
231+
}
232+
else
233+
{
234+
heldCall.SetVoidResult();
235+
}
225236
// The render's continuation was queued on the renderer's dispatcher by the line above, so a
226237
// no-op behind it only completes once that continuation has run.
227238
await component.InvokeAsync(() => { });
@@ -242,6 +253,81 @@ public async Task DisposeAsync_WhileARenderAwaitsTheJSInstance_ThatRenderGoesNoF
242253
context.Renderer.UnhandledException.IsCompleted.ShouldBeFalse();
243254
}
244255

256+
[Theory]
257+
[InlineData(typeof(JSException), LogLevel.Debug)]
258+
[InlineData(typeof(JSDisconnectedException), LogLevel.Debug)]
259+
[InlineData(typeof(ObjectDisposedException), LogLevel.Debug)]
260+
[InlineData(typeof(TaskCanceledException), LogLevel.Debug)]
261+
[InlineData(typeof(InvalidOperationException), LogLevel.Warning)]
262+
public async Task ScrollToTop_StillInFlightWhenTheComponentIsDisposed_CompletesWithoutThrowing(Type failure, LogLevel expectedLogLevel)
263+
{
264+
// Arrange
265+
using var context = CreateContext();
266+
var log = new CapturingLogger();
267+
context.Services.AddSingleton<ILogger<AdaptiveVirtualize<string>>>(log);
268+
var instance = new FakeJS().Throwing("scrollToIndex", (Exception) Activator.CreateInstance(failure, "scroll failed")!);
269+
instance.Hold("scrollToIndex");
270+
var module = new FakeJS().Returning("create", instance);
271+
context.Services.AddSingleton<IJSRuntime>(new FakeJS().Returning("import", module));
272+
var component = RenderWith(context, ["a"]);
273+
274+
// Act
275+
// Fired off and not awaited, the way an app scrolls a list back to the top after a filter change.
276+
Task scroll = null!;
277+
await component.InvokeAsync(() => { scroll = component.Instance.ScrollToTop(); });
278+
await component.InvokeAsync(async () => await component.Instance.DisposeAsync());
279+
instance.Answer("scrollToIndex");
280+
await component.InvokeAsync(() => { });
281+
282+
// Assert
283+
// Thrown, it would reach a caller that did not await it only as an unobserved task exception.
284+
// Checked by status: Should.NotThrowAsync lets a canceled task through as "did not throw".
285+
scroll.Status.ShouldBe(TaskStatus.RanToCompletion);
286+
log.Entries.ShouldContain(entry => entry.Level == expectedLogLevel && entry.Exception != null && entry.Exception.GetType() == failure);
287+
}
288+
289+
[Fact]
290+
public async Task GetSnapshot_StillInFlightWhenTheComponentIsDisposed_ReturnsNothing()
291+
{
292+
// Arrange
293+
using var context = CreateContext();
294+
var instance = new FakeJS().Throwing("getSnapshot", new JSException("JS object instance with ID 20 does not exist (has it been disposed?)"));
295+
instance.Hold("getSnapshot");
296+
var module = new FakeJS().Returning("create", instance);
297+
context.Services.AddSingleton<IJSRuntime>(new FakeJS().Returning("import", module));
298+
var component = RenderWith(context, ["a"]);
299+
300+
// Act
301+
Task<VirtualizerSnapshot?> query = null!;
302+
await component.InvokeAsync(() => { query = component.Instance.GetSnapshot(); });
303+
await component.InvokeAsync(async () => await component.Instance.DisposeAsync());
304+
instance.Answer("getSnapshot");
305+
await component.InvokeAsync(() => { });
306+
307+
// Assert
308+
// The same answer as a query made after disposal.
309+
query.Status.ShouldBe(TaskStatus.RanToCompletion);
310+
(await query).ShouldBeNull();
311+
}
312+
313+
[Fact]
314+
public async Task ScrollToTop_JSFailsWhileTheComponentIsAlive_Throws()
315+
{
316+
// Arrange
317+
using var context = CreateContext();
318+
var instance = new FakeJS().Throwing("scrollToIndex", new JSException("scroll failed"));
319+
var module = new FakeJS().Returning("create", instance);
320+
context.Services.AddSingleton<IJSRuntime>(new FakeJS().Returning("import", module));
321+
var component = RenderWith(context, ["a"]);
322+
323+
// Act
324+
var scroll = () => component.InvokeAsync(() => component.Instance.ScrollToTop());
325+
326+
// Assert
327+
// Only disposal makes a failure expected; before it, it may be a genuine bug.
328+
await Should.ThrowAsync<JSException>(scroll);
329+
}
330+
245331
[Fact]
246332
public async Task DisposeAsync_WhileTheModuleIsBeingImported_ReleasesItAndNeverCreatesAnInstance()
247333
{

0 commit comments

Comments
 (0)