hardkoded / puppeteer-sharp

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

NetworkManager and others create unobserved exceptions #2642

Open ECrownofFire opened 4 months ago

ECrownofFire commented 4 months ago

This is mostly relevant when using logging solutions such as Sentry that hook into TaskScheduler.UnobservedTaskException. Unobserved tasks can be a major source of bugs, so I'd really like to keep noisiness in that event to a minimum.

On this line in NetworkManager, an exception is set on a Task that can only be observed if that response's body is attempted to be read.

There are a few other places where unobserved exceptions are created as well, such as here in IsolatedWorld.

Exceptions can be forcefully marked as observed with a snippet such as this, placed before TrySetException.

TaskCompletionSource taskWrapper;
// [...]

// Ensure that the exception is always marked as observed
taskWrapper.Task.ContinueWith(
    (t) => { _ = t.Exception; },
    CancellationToken.None,
    TaskContinuationOptions.OnlyOnFaulted,
    TaskScheduler.Default);
taskWrapper.TrySetException(new Exception("Something went wrong"));

It's not really ideal, but something like this should probably be placed everywhere that an exception is set that doesn't need to be observed. Several of these are basically just asynchronous getters that only want to throw an error when the result is unavailable, so observing them is unnecessary if they're not called.

kblok commented 4 months ago

I had many attempts to resolve this here and here but I found no solutions.

PRs are welcome.