Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature] Avoid async delegates #2669

Closed
mathiasi opened this issue Aug 16, 2023 · 9 comments
Closed

[Feature] Avoid async delegates #2669

mathiasi opened this issue Aug 16, 2023 · 9 comments
Labels

Comments

@mathiasi
Copy link

We are trying to use Page.RequestFinished however the advocated approach involves async delegates which triggers this analyzer and it does appear to be causing some issues for us but since the request only has ResponseAsync it is a bit hard to avoid using async inside. It would be if there was a way around this or to have a synchronous way of getting the response. Afterall that is probably what most people are interested in when using RequestFinished specifically

@kblok
Copy link
Contributor

kblok commented Aug 16, 2023

@mxschmitt I was about to recommend RunAndWaitForRequestFinishedAsync. But the problem I found there is that the Predicate is a Func<Request, bool> and it would be nice to have a Func<Request, Task<bool>> option.

@mxschmitt
Copy link
Member

Sounds like #2576

@mathiasi
Copy link
Author

I cannot speak to the similarity as we are interested in specifically the callback. We are using it to keep track of all the API calls that are triggered by the Playwright tests and then know what data to clean up after the tests is complete. We are interested in some response data as it contains ID's and therefore we have to look at the response which unfortunately for us is only available as an async operation.
If only we could it synchronously like this it would serve our needs:

page.RequestFinished += (page, request) =>
  {
    _tracking.Save(request.Url, request.Response());
  }

@mxschmitt
Copy link
Member

I see, there is unfortunately no sync way of getting the response. I recommend either listening on "Response" event instead, which is fired after RequestFinished. Would that work for you?

@mathiasi
Copy link
Author

@mxschmitt Maybe I have misunderstood you but the Response event gives a Response object which is also very async-focused. Did I miss a way to get the response body in a synchronous way on Response? It seems to me that I'll be facing the same problem whether we use Response or RequestFinished events.

@mxschmitt
Copy link
Member

Oh ok, then there is no way, then maybe collect the Request/Response objects and bulk process them at the end of your tests or before you navigate?

@mathiasi
Copy link
Author

Yeah I'm contemplating that solution too. Even though that will also have a downside: https://github.com/Microsoft/vs-threading/blob/main/doc/analyzers/VSTHRD003.md

@mxschmitt
Copy link
Member

mxschmitt commented Aug 18, 2023

Sounds then unfortunately a won't fix to me, since we can't change our model to have sync getters (architecture wise).

I'd recommend for now to disable the specific analyzer rules.

@mxschmitt
Copy link
Member

Closing as per above, since this is more of a linter problem rather than an actual bug/feature request in Playwright.

@mxschmitt mxschmitt closed this as not planned Won't fix, can't repro, duplicate, stale Aug 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants