puppeteer-sharp
puppeteer-sharp copied to clipboard
Fixes bug #1016: Unhandled TargetClosedException
It solves the issue by collecting the lingering tasks, and manually making them throw so that the UnobservedTaskException event doesn't see them anymore.
I tried two different methods (you can see there are two commits), but it's weird, because the tests run fine in my machine for either of them, but AppVeyor doesn't seem happy about them.
It may be a little harder than I thought.
@R2D221 yes, not easy. But AppVeyor says which code is good and which isn't :p
If you want to keep trying stuff and pushing code, go for it, I have no problem with that.
What do you think about adding these 2 lines in this PR? https://github.com/kblok/puppeteer-sharp/pull/1152/files#diff-408613782162a72f6aace17be118570aR36
If we want to guarantee that we are observing all exceptions we would need to test it across the board.
I think it makes sense, but if you throw an exception in the unobserved event, I'm not exactly sure where it will throw it.
The approach I was using is collecting the errors in a list, forcing GC to collect, and then see if the list is empty. I don't know if this can be set up globally for all tests.
That's true. In my PR I added a Collect
as you did. Maybe we could add that as a safety net when you finish this feature?
We should never get that. Or at list a Console.WriteLine
to get that in CI?
Sure. Let me see how this run goes, and maybe we can add it as well.
One more thing. Could you test this on BrowserDisconnectTests.ShouldRejectNavigationWhenBrowserCloses? That was my headache on #1152
I was busy on the weekend, but I tried to take a look at that.
I saw you created a variable LazyBodyLoadedTaskWrapper
, but I think that's not the best way to solve it. Instead of deferring the creation of the task, we should move the exception logic outside of it. Let the task resolve with True or False as a success flag, and only throw an exception when it resolved as False (in this case, when we try to get the response body of a redirect).
I'll push another commit with the way I'm solving it. Just give me a moment.
(The last commit doesn't have a test associated. I'm kind of busy at work, but you get the idea)
In general, I think the way to go is checking how the tasks are failing unobserved, and think around it and only fail after we observe them.
Looking good, looking good. Thanks for your hard work!
Hey there @kblok @R2D221 !
Do you have any status for the PR ? It seems that I have a problem and these changes could fix it. Described at issue #1801 :smile:
Cheers!
@HunteRoi Hi!
Well, I was working on this a while ago, but if I recall correctly, I couldn't completely fix it. So I'd have to see what's missing.
Keep us in the loop please :smile: