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

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

Closed
thowo opened this issue May 14, 2024 · 8 comments

Comments

@thowo
Copy link

thowo commented May 14, 2024

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
Copy link

elmar56 commented May 14, 2024

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

@thowo
Copy link
Author

thowo commented May 14, 2024

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

@boghyon
Copy link
Contributor

boghyon commented May 15, 2024

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:

  • URL of a minimal sample
  • Steps to reproduce the issue
  • Firefox version: Mozilla has released FF 126 just yesterday. Could you try with an older FF version or the Nightly version?
  • OS version
  • Is there any known issue from Dynatrace?

@thowo
Copy link
Author

thowo commented May 15, 2024

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
Copy link
Author

thowo commented May 15, 2024

For your additional questions:

  • We tried different Firefox versions on Linux and windows, but all behave the same. we have created new firefox profiles, cleaned caches, hung out hours in cors analysis => nothing worked

  • We had Dynatrace support involved here and they pointed us to syncXHRFix.js. The DT Support mentioned DT does nothing else than ordinary xmlhttprequest (but in case of firefox this will be rewritten by syncXHRFix)

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

@thowo
Copy link
Author

thowo commented May 16, 2024

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
Copy link
Contributor

boghyon commented May 19, 2024

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:

  • the fetch API only if the current browser is not FF (Determined by user agent sniffing).
  • the XMLHttpRequest instances for FF since the fetch API's keepalive option is still not supported in FF yet — Mozilla just started working on it.
  • I remember seeing an option alwaysBeacon in the script code which, IMO, should be the preferred sending strategy if possible (Cf. Improving page dismissal in synchronous XMLHttpRequest() | web.dev). The Beacon API is supported by all modern browsers incl. FF.

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"):

  • works either without the Dynascript making the open a bound function or without the syncXHRFix.
  • fails if both are applied.

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. #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
Copy link
Author

thowo commented May 22, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants