SAP / openui5

OpenUI5 lets you build enterprise-ready web applications, responsive to all devices, running on almost any browser of your choice.
http://openui5.org
Apache License 2.0
2.9k stars 1.23k forks source link

Advise needed: Firefox does not send xhr because of syncXHRFix.js and XHRInterceptor.js #4049

Closed thowo closed 1 month ago

thowo commented 1 month ago

Hello openui5 Team,

we currently have the situation that we want to measure the end user performance in a UI5 project with the help of dynatrace. For this purpose, a javascript (ruxitagent) is loaded into the browser and corresponding measurements of the response time behavior of the UI5 application are created based on the W3C performance timings. The measurement results are sent via xhr to a SaaS instance for further analysis and reporting.

At this point, the JS files syncXHRFix.js and XHRInterceptor.js available in UI5 prevent the transmission of the measurement results, but only in the Firefox browser. According to the comment in the source code of the files, these were installed about 5 years ago as a workaround to stabilize the handling of synchronous requests in Firefox. In Edge and Chrome, however, the XHRs are sent successfully.

My question is: Are these old patches still needed today? Is it possible to disable the interception of XHR requests in Firefox by bypassing the syncXHRFix / XHRInterceptor? Is there a config option for this?

Any further idea on how to work around the Firefox xhr issue is highly welcome.

Thank you in advance, best regards, Tom

elmar56 commented 1 month ago

We're also affected, I am looking forward to the solution. Cheers! Elmar

thowo commented 1 month ago

In addition, we are using version 1.96 but in latest stable 1.120 seems to be the same

boghyon commented 1 month ago

At this point, the JS files syncXHRFix.js and XHRInterceptor.js available in UI5 prevent the transmission of the measurement results, but only in the Firefox browser.

Could you elaborate more on that?

[...] patches still needed today?

The referenced Firefox bug https://bugzilla.mozilla.org/show_bug.cgi?id=697151 is still not resolved.

Could you please share:

thowo commented 1 month ago

Hello and thanks for taking care on this!

Please find a demo of the issue at:

https://vserver.wollner.cloud/

it contains the "ui5 demo todo app" with dynatrace ruxitagent.js integrated and it shows the firefox issue directly.

If you hit the page with chrome or edge everything works as expected, measurement beacons are sent out to the dynatrace saas instance at https://bf17584hvk.bf.dynatrace.com, but firefox cannot send out because of syncXHRFix.js.

In other web application frameworks out, we did not experience such a firefox behaviour, only ui5 behaves different here.

Thanks for all your thoughts, best regards, Tom

thowo commented 1 month ago

For your additional questions:

If you need more information please let me know. thanks in advance, best regards, Tom

thowo commented 1 month ago

Here is a screenshot of the browser console on Firefox where one can see the issue.

5f1c226180c80dc0_complete.js is the dyntrace ruxitagent loaded from cdn (take care on Firefox Tracking Protection (must be disabled here) if you want to give it a try.

jsconsole-syncXHRFix

boghyon commented 1 month ago

Thanks for sharing the minimal demo app. It looks like the expected Dynatrace requests are working now even in Firefox.

Last time I checked, the reports were still not being sent in FF, and I could see in the unminified Dynatrace script a function named getBeaconSendingStrategy which seemed to determine which web APIs the script should use for sending the reports to the server. Depending on the current browser, the sending strategy was based on:

I'd suggest reaching out to the Dynatrace support to learn more about the sending strategies and whether the Beacon API based strategy can be used for FF users.


Regarding

[...] The DT Support mentioned DT does nothing else than ordinary XMLHttpRequest [...].

Here is a summarized version of what is actually happening:

// OpenUI5: syncXHRFix
globalThis.XMLHttpRequest = new Proxy(globalThis.XMLHttpRequest, {
  construct(TargetClass, args) {
    const xhr = new TargetClass(...args);
    return new Proxy(xhr, {
      get(target, propertyName) {
        const vProp = target[propertyName];
        return propertyName === "open" ? function() {
          vProp.apply(target, arguments); // fails due to Dynatrace making vProp a bound function
        } : vProp;
      }
    });
  }
});

const myXHR = new XMLHttpRequest()

// Dynatrace:
myXHR.open = Function.prototype.bind.call.apply(Function.prototype.bind, [XMLHttpRequest.prototype.open, myXHR]); // from functionBind <-- works without
myXHR.open("POST", "https://example.com"); // In Firefox only, since no fetch API w/ keepalive

In my view, what Dynatrace does in functionBind is not really "ordinary".

You can try the above code snippet anywhere. We can see that myXHR.open("POST", "https://example.com"):

The syncXHRFix is, unfortunately, still required since the referenced Firefox bug is still reproducible. Since the FF issue is not resolved yet, I don't see how the syncXHRFix could be removed while some existing legacy applications or even some parts of the framework still rely on sync XHRs (Cf. https://github.com/SAP/openui5/issues/2345).

I was able to confirm that bypassing the functionBind call in Dynatrace resolves sending the XHRs in Firefox. But now, the minimal demo app seems to be sending the reports expectedly anyhow. :)

thowo commented 1 month ago

Hello,

thank you very much for your effort and your detailed explanation on this topic. We got some reply from the DT devs, which we want to let you know:

<<< Quotation of DT Support starts here The problem is that fetch on Firefox is not as reliable as XHR. Firefox does not (yet, it is currently being worked on) support fetch-keepalive, which is supposed quite reliable, even if you leave the page. Firefox does support fetch itself, but it is not as reliable as XHRs (it is fine during normal operation of the page, but if you send a beacon and then navigate away from the page, XHRs get cancelled less than fetch requests (maybe 5-10 % difference, but we have noticed some issues and therefore decided to keep the XHR beacon sending in the jsagent for Firefox, until fetch keep-alive hopefully fixes this problem)

We also don't have a config flag that allows you to switch to the fetch sending and we are not going to switch to the fetch strategy, as this would have detremental effects for all other customers.

EoQ

The fact that the demo app now works is more or less a coincidence and depends on whether dynatrace or sap-ui loads faster. If sap-ui is used with local node_modules and caching, dynatrace is too late via cdn and is overloaded by syncXHRFix.

thanks again, best regards, Tom

boghyon commented 6 days ago

the referenced Firefox bug is still reproducible.

The bug is now fixed (Tested with 129.0a1 2024-07-01). But it may take a while until the syncXHRFix can be finally removed since UI5 has to support FF ESR as well, and it is not clear yet whether the fix lands in ESR. I'll keep this issue updated.