webdriverio-community / wdio-intercept-service

🕸 Capture and assert HTTP ajax calls in webdriver.io
MIT License
104 stars 33 forks source link

Intercepting XMLHttpRequests is broken #776

Open muhserks opened 6 months ago

muhserks commented 6 months ago

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 (https://github.com/webdriverio-community/wdio-intercept-service/blob/b2df722aa9a138cab4408513582470089e9550ba/lib/interceptor.js#L118) 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 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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

muhserks commented 6 months ago

Well, simplest example is clearing all the requests. Referring to https://github.com/webdriverio-community/wdio-intercept-service/issues/249#issuecomment-1066201741 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 commented 6 months ago

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 commented 6 months ago

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.