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

Intercepting XMLHttpRequests is broken #776

Open
muhserks opened this issue Mar 4, 2024 · 6 comments
Open

Intercepting XMLHttpRequests is broken #776

muhserks opened this issue Mar 4, 2024 · 6 comments
Labels

Comments

@muhserks
Copy link
Contributor

muhserks commented Mar 4, 2024

Intercept Version : 4.4.0
WDIO Version: 8.32.3

I do not fully understand why it is failing, but when intercepting XMLHttpRequests, they will be duplicated inside the session storage.
Each intercepted request will create additional ones inside the array in session storage. Means, the 3rd request will create 3 entries, the 4th will create 4 entries ... and so on ... All have same URL, method etc ...
Looking at the stacktrace inside XMLHttpRequest.prototype.send shows that it somehow got replaced multiple times. Each send will call an additional send.

The only thing I found to fix this is to check at the beginning of replaceXHR (

function replaceXHR() {
) if XMLHttpRequest.prototype.send is already overwritten:

if (XMLHttpRequest.prototype.send.toString().includes("[native code]")==false) {
    return;
}

But still, I am a bit clueless here if that would be the fix here. @tehhowch Would you mind to share some wisdom here?

I will try to prepare something reproducable ...

@tehhowch
Copy link
Contributor

tehhowch commented Mar 4, 2024

A better check would be to have setupInterceptor only do the replaces if its own flag has not been set - it's possible for page code to do its own polyfills / middleware.

The issue you are experiencing is not due to the library itself - otherwise our unit tests with multiple requests would be failing.

@muhserks
Copy link
Contributor Author

muhserks commented Mar 4, 2024

Thanks for answering. I pushed an example here: https://github.com/muhserks/wdio-intercept-service/tree/reproduce-776

In my WDIO framework, I am only waiting that a request was completed after a click. I call always setupInterceptor and waitUntil browser.hasPendingRequests() is false. But when 2nd request is done, browser.hasPendingRequests() is always true.
The weird thing is, this works with wdio intercept V4.2.0.

Anyway, Let me see if I can figure out a way to avoid the replacement in setupInterceptor.

@tehhowch
Copy link
Contributor

tehhowch commented Mar 5, 2024

Why do you repeatedly call setupInterceptor, when the interceptor is already loaded?

@muhserks
Copy link
Contributor Author

muhserks commented Mar 5, 2024

Well, simplest example is clearing all the requests. Referring to #249 (comment) it was the suggested way and it makes sense.

But my case is that in the application, same page, when you select certain things, ajax requests are made for each click/select/whatever. If I don't wait for it to be finished, subsequent actions will fail. There is no DOM selection possibility to do it in any other way.
Bottom line is, I have this in my framework (setupInterceptor and waitUntil browser.hasPendingRequests() is false) for a couple of methods which are getting called throughout the tests. I do not set up the interceptor for each test seperately, it is just setup when it is used (and hopefully disabled afterwards when I can finally update ....).

Putting the interceptor stuff directly into the tests is no option. Keeping track on my side if it is already setup or not is also not ideal imo.

@tehhowch
Copy link
Contributor

tehhowch commented Mar 5, 2024

I don't think I had thought through the ramifications of calling setupInterceptor multiple times when I made that post, unfortunately.

If you're embedding this in your framework rather than the tests that need it, I suggest ensuring it's done after your framework has executed browser.url rather than as a part of clicks. You could also abuse some private implementation details of this service and use browser.execute to set window.__webdriverajax.requests = []; e.g. as your own clearSpiedRequests method.

(That the application you're testing both requires the network to settle before you can do the next thing, and provides no UI feedback as to when the network has settled, seems like terrible UX and something you should raise with your developer team.)

Providing public API to clear the captured requests is certainly a reasonable request, btw.

@muhserks
Copy link
Contributor Author

muhserks commented Mar 7, 2024

If you navigate through your application and change pages, you need to setup the interceptor again and I do not want to do that for each link I may click. I only execute browser.url once to get to the login screen. It is also not the case that I always need the interceptor on that page with ajax requests.
Even though I implemented disable interceptor (shame on me), enabling it is missing. Calling setupInterceptor to enable it again will not work as we already figured out (at least if you stay on the same page).
Clearing the requests is not only window.__webdriverajax.requests = [];. The session store needs also to be cleared. Otherwise you will get weird results and you could also hit the limit again.

I did an implementation now on my side via browser execute to take care of all 3 cases.

For me, these changes do make sense and it could also avoid surprises for other people using the interceptor. But that is just my opinion.
If you are cool with these changes, I can create a PR and we could continue from there. It is up to you.

With that being said, I am still a bit confused about window.__webdriverajax.requests VS the requests in sessionstorage and how they are used. getRequests() will use sessionstorage (if available), but hasPendingRequests will not rely on session storage. window.__webdriverajax.requests can differ from sessionstorage after a page change, but gets cleared while calling the needed setup again. It will also differ if URLs are excluded, because URLs are only excluded while pushing into session storage
So why is the sessionstorage even needed? I am probably missing here something, but wouldn't it be sufficient to just use the namespace?

Oh, and thanks for the feedback.

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

2 participants