hardkoded / puppeteer-sharp

Headless Chrome .NET API
https://www.puppeteersharp.com
MIT License
3.42k stars 446 forks source link

Disposal of CdpElementHandle.Handle #2698

Closed jnyrup closed 4 months ago

jnyrup commented 4 months ago

Description

I noticed that in CdpElementHandle.DisposeAsync we don't await the disposal of Handle.DisposeAsync(). Is that intended? If yes, I suggest we assign it to a discard to make it more explicit that we don't want to await it.

https://github.com/hardkoded/puppeteer-sharp/blob/85aa4b0746d62e05a02059e511693d4571993f04/lib/PuppeteerSharp/Cdp/CdpElementHandle.cs#L147-L159

Complete minimal example reproducing the issue

Complete means the code snippet can be copied into a unit test method in a fresh C# project and run. Minimal means it is stripped from code not related to reproducing the issue.

E.g.

// Arrange
var options = new LaunchOptions { /*  */ };
var chromiumRevision = BrowserFetcher.DefaultRevision;
var browser = await Puppeteer.LaunchAsync(options, chromiumRevision);
var page = browser.NewPageAsync();

// Act
...

// Assert
...
kblok commented 4 months ago

That's wrong. Good catch. https://github.com/hardkoded/puppeteer-sharp/pull/2699