chore: dispose APIRequestContext on BrowserContext close

microsoft · Jul 29, 2024 · c0f5923 · c0f5923
1 parent a5f629d
commit c0f5923
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 2 deletions.
diff --git 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<PlaywrightException>(() => Context.APIRequest.GetAsync(Server.EmptyPage));
+ StringAssert.Contains("Test ended.", exception.Message);
+ }
+
  private async Task ForAllMethods(IAPIRequestContext request, Func<Task<IAPIResponse>, 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
@@ -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<string, object>
+ {
+ ["reason"] = reason,
+ }).ConfigureAwait(false);
+ }
+ catch (Exception e) when (DriverMessages.IsTargetClosedError(e))
+ {
+ // Swallow exception
+ }
  _tracing.ResetStackCounter();
  }
 
@@ -79,6 +96,10 @@ internal async Task<IAPIResponse> InnerFetchAsync(IRequest request, string urlOv
  [MethodImpl(MethodImplOptions.NoInlining)]
  public async Task<IAPIResponse> 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
@@ -46,7 +46,7 @@ internal class BrowserContext : ChannelOwner, IBrowserContext
  private readonly Tracing _tracing;
  private readonly Clock _clock;
  internal readonly HashSet<IPage> _backgroundPages = new();
- internal readonly IAPIRequestContext _request;
+ internal readonly APIRequestContext _request;
  private readonly Dictionary<string, HarRecorder> _harRecorders = new();
  internal readonly List<IWorker> _serviceWorkers = new();
  private List<RouteHandler> _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 () =>
  {