From c0f5923a3d85a1acc883b7b85d6db491eee63eb8 Mon Sep 17 00:00:00 2001 From: Max Schmitt Date: Mon, 29 Jul 2024 10:14:22 +0200 Subject: [PATCH] chore: dispose APIRequestContext on BrowserContext close --- .../BrowserContextFetchTests.cs | 8 +++++++ src/Playwright/Core/APIRequestContext.cs | 23 ++++++++++++++++++- src/Playwright/Core/BrowserContext.cs | 8 ++++++- 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/src/Playwright.Tests/BrowserContextFetchTests.cs b/src/Playwright.Tests/BrowserContextFetchTests.cs index 400c636a8..b8d31273a 100644 --- a/src/Playwright.Tests/BrowserContextFetchTests.cs +++ b/src/Playwright.Tests/BrowserContextFetchTests.cs @@ -884,6 +884,14 @@ public async Task ShouldSupportRepeatingNamesInMultipartFormData() Assert.AreEqual(200, response.Status); } + [PlaywrightTest("browsercontext-fetch.spec.ts", "should not work after context dispose")] + public async Task ShouldNotWorkAfterContextDispose() + { + await Context.CloseAsync(new() { Reason = "Test ended." }); + var exception = await PlaywrightAssert.ThrowsAsync(() => Context.APIRequest.GetAsync(Server.EmptyPage)); + StringAssert.Contains("Test ended.", exception.Message); + } + private async Task ForAllMethods(IAPIRequestContext request, Func, Task> callback, string url, APIRequestContextOptions options = null) { var methodsToTest = new[] { "fetch", "delete", "get", "head", "patch", "post", "put" }; diff --git a/src/Playwright/Core/APIRequestContext.cs b/src/Playwright/Core/APIRequestContext.cs index c27bc1204..44111f73f 100644 --- a/src/Playwright/Core/APIRequestContext.cs +++ b/src/Playwright/Core/APIRequestContext.cs @@ -38,6 +38,7 @@ namespace Microsoft.Playwright.Core; internal class APIRequestContext : ChannelOwner, IAPIRequestContext { internal readonly Tracing _tracing; + private string _closeReason; internal APIRequest _request; @@ -49,7 +50,23 @@ public APIRequestContext(ChannelOwner parent, string guid, APIRequestContextInit [MethodImpl(MethodImplOptions.NoInlining)] public async ValueTask DisposeAsync() { - await SendMessageToServerAsync("dispose").ConfigureAwait(false); + await DisposeAsync(null).ConfigureAwait(false); + } + + internal async ValueTask DisposeAsync(string reason) + { + _closeReason = reason; + try + { + await SendMessageToServerAsync("dispose", new Dictionary + { + ["reason"] = reason, + }).ConfigureAwait(false); + } + catch (Exception e) when (DriverMessages.IsTargetClosedError(e)) + { + // Swallow exception + } _tracing.ResetStackCounter(); } @@ -79,6 +96,10 @@ internal async Task InnerFetchAsync(IRequest request, string urlOv [MethodImpl(MethodImplOptions.NoInlining)] public async Task FetchAsync(string url, APIRequestContextOptions options = null) { + if (!string.IsNullOrEmpty(_closeReason)) + { + throw new PlaywrightException(_closeReason); + } options ??= new APIRequestContextOptions(); if (options.MaxRedirects != null && options.MaxRedirects < 0) diff --git a/src/Playwright/Core/BrowserContext.cs b/src/Playwright/Core/BrowserContext.cs index 3d27b560d..9414a42d3 100644 --- a/src/Playwright/Core/BrowserContext.cs +++ b/src/Playwright/Core/BrowserContext.cs @@ -46,7 +46,7 @@ internal class BrowserContext : ChannelOwner, IBrowserContext private readonly Tracing _tracing; private readonly Clock _clock; internal readonly HashSet _backgroundPages = new(); - internal readonly IAPIRequestContext _request; + internal readonly APIRequestContext _request; private readonly Dictionary _harRecorders = new(); internal readonly List _serviceWorkers = new(); private List _routes = new(); @@ -320,6 +320,12 @@ public async Task CloseAsync(BrowserContextCloseOptions options = default) } _closeReason = options?.Reason; CloseWasCalled = true; + await WrapApiCallAsync( + async () => + { + await _request.DisposeAsync(options?.Reason).ConfigureAwait(false); + }, + true).ConfigureAwait(false); await WrapApiCallAsync( async () => {