EandrewJones commented on code in PR #453:
URL: https://github.com/apache/flagon-useralejs/pull/453#discussion_r1596079285


##########
src/sendLogs.js:
##########
@@ -62,12 +62,18 @@ export function sendOnInterval(logs, config) {
 export function sendOnClose(logs, config) {
   window.addEventListener("pagehide", function () {
     if (config.on && logs.length > 0) {
-      // NOTE: sendBeacon does not support auth headers,
-      // so this will fail if auth is required.
-      // The alternative is to use fetch() with keepalive: true
-      // 
https://developer.mozilla.org/en-US/docs/Web/API/Navigator/sendBeacon#description
-      // https://stackoverflow.com/a/73062712/9263449
-      navigator.sendBeacon(config.url, JSON.stringify(logs));
+
+      let header_options = config.authHeader ? 
+        {'Content-Type': 'application/json;charset=UTF-8', 'Authorization': 
config.authHeader} 
+        : {'content-type': 'application/json;charset=UTF-8'};
+
+      fetch(config.url, {

Review Comment:
   I recommend we stick with old school for now until we add async/await 
everywhere. And since we're not doing anything with the response, all we need 
is the `.catch()`



##########
src/sendLogs.js:
##########
@@ -62,12 +62,18 @@ export function sendOnInterval(logs, config) {
 export function sendOnClose(logs, config) {
   window.addEventListener("pagehide", function () {
     if (config.on && logs.length > 0) {
-      // NOTE: sendBeacon does not support auth headers,
-      // so this will fail if auth is required.
-      // The alternative is to use fetch() with keepalive: true
-      // 
https://developer.mozilla.org/en-US/docs/Web/API/Navigator/sendBeacon#description
-      // https://stackoverflow.com/a/73062712/9263449
-      navigator.sendBeacon(config.url, JSON.stringify(logs));
+
+      let header_options = config.authHeader ? 
+        {'Content-Type': 'application/json;charset=UTF-8', 'Authorization': 
config.authHeader} 
+        : {'content-type': 'application/json;charset=UTF-8'};
+
+      fetch(config.url, {

Review Comment:
   fetch is async. We either need to switch to Javscript's async/await 
syntactic sugar or use the old school `.then(...).catch(...)` syntax.
   
   See: 
[https://rapidapi.com/guides/fetch-api-async-await](https://rapidapi.com/guides/fetch-api-async-await)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscr...@flagon.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@flagon.apache.org
For additional commands, e-mail: notifications-h...@flagon.apache.org

Reply via email to