You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flagon.apache.org by GitBox <gi...@apache.org> on 2021/03/09 02:33:23 UTC

[GitHub] [incubator-flagon-useralejs] poorejc opened a new issue #50: Last Log Drops from Log Queue when exiting page | specific configs

poorejc opened a new issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50


   Navigating from a page to another should trigger "sendOnClose" which flushes the log queue when a page is closed. 
   
   user report that at transmittalInterval >50ms and logcountthreshold=1, the last log (e.g., 'click') isn't being transmitted.


----------------------------------------------------------------
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc commented on issue #50: Last Log Drops from Log Queue when exiting page | specific configs

Posted by GitBox <gi...@apache.org>.
poorejc commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-801268658


   Some forensics: sendOnClose was not functioning, at all.
   
   It appears there were two failures:
   
   1. sendOnClose doesn't execute because it is conditional on !config.on which is equivalent to not-started (as in UserALE.js) is not started. This is short-hand for state == interactive (see ./src/main.js). I don't know why this doesn't work, but for page lifecycle events as fast as navigations, it could be that the function breaks as userale.js is infact unloaded and the config object doesn't exist anymore.
   
   2. sendOnClose uses 2 methods to try to send logs:
   --  if (navigator.sendBeacon) {
       window.addEventListener('unload', function() {
         navigator.sendBeacon(config.url, JSON.stringify(logs));
       });
     } else {
   --  window.addEventListener('beforeunload', function() {
         if (logs.length > 0) {
           sendLogs(logs, config, 1);
   Neither of these appear working using the /example page: first the /example page doesn't init a navigator.sendbeacon on the page. second, 'beforeunload' is unreliable. Though I didn't run all permutations of removing notstarted logic and using each of these options indepenedently. 
   
   I can't get it to work.


----------------------------------------------------------------
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.

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



[GitHub] [incubator-flagon-useralejs] confusingstraw commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
confusingstraw commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-806004426


   @UncleGedd for future reference, i'd recommend checking out using these:
   [https://sinonjs.org/releases/v10.0.0/stubs/](https://sinonjs.org/releases/v10.0.0/stubs/)
   [https://sinonjs.org/releases/v10.0.0/spies/](https://sinonjs.org/releases/v10.0.0/spies/)
   when dealing with faking functionality, rather than setting up your own stuff like `called = true`. you can use these helpers to do some interesting stuff.


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] UncleGedd edited a comment on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
UncleGedd edited a comment on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-806668259


   Ok, so I've tried writing a cypress/journey test for `sendBeacon` and it's pretty flaky, but manual testing works in the browser (Chrome) consistently though


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
poorejc commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-802499142


   Removed sendOnRefresh from Submit event handling:
   
   ```
     Object.keys(refreshEvents).forEach(function(ev) {
       document.addEventListener(ev, function(e) {
         packageLog(e, events[ev]);
       }, true);
     });
   ```
   sendOnRefresh was triggering duplicate events. Given that Submit events trigger a refresh which switches the page into a 'hidden' visibility state, sendOnClose now correctly pushes submit chain of events (change, click, submit, load).


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc edited a comment on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
poorejc edited a comment on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-808621268


   @confusingstraw you've always been someone I consider imminently sane.
   
   Sounds like the right way to go is [3] Websockets @confusingstraw makes a compelling case.
   - it solves the connectivity and event handling problems
   - it's something that's pretty ubiquitous
   - it aligns with previous user interest
   - the consequences of the "false positive" is that everything in the log queue is sent to back-end and queue is flushed. That results in neither duplicate logs nor dropped logs. Seems like a low severity risk
   - the big drawback is that we have to manage/maintain supporting plugins for ELK and probably provide some other examples for plugins to a few other datastores (e.g., postgress). That's not too crazy.
   
   [1,2,4,5] seem like reaches and [5] seems like it would introduce other headaches. 
   
   If nothing else heard (feel free to add opinion here @kevbrowndev, @UncleGedd), I can put together some new tickets to cover this. Could use some help on this one.
   
   Other thing would like opinions on:
   
   To do this right, we should also make sure to mod and test our ELK stack and run full end-to-end integration tests. That could take a little longer. Is it worth doing a little more testing on this branch as is, pushing into next minor release (2.2.0) as a 'hot-fix', then doing the refactoring on sendLogs to websockets as the following release (2.3.0). Would be nice to make sure than NPM users have something incrementally better than current sendOnClose behavior (which doesn't behave at all?) Opinions?
   
   


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] UncleGedd commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
UncleGedd commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-808762700


   Regarding the Websockets option, does this mean we would need to maintain the logs queue server-side as well? That way, whenever the connection is lost/interrupted, the server has a way of knowing what logs to flush?
   
   I'd like to spike on the web worker/service worker option a bit (will work on it tomorrow). The goal wouldn't be for the web worker to listen to additional events, but rather use the `visibilitychange ` event in the main browser thread to perform a [postMessage](https://developer.mozilla.org/en-US/docs/Web/API/Worker/postMessage) and have the worker perform the HTTP request. My assumption is that the worker will be able to perform the request in a more reliable way than `sendBeacon`


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] kevbrowndev commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
kevbrowndev commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-806866064


   Fwiw, I've seen the exact same thing. Frustrating!
   
   
   
   On Thu, Mar 25, 2021, 8:41 AM UncleGedd ***@***.***> wrote:
   
   > Ok, so I've tried writing a cypress/journey test for sendBeacon and it's
   > pretty flaky. It does work in the browser (Chrome) consistently though
   >
   > —
   > You are receiving this because you are subscribed to this thread.
   > Reply to this email directly, view it on GitHub
   > <https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-806668259>,
   > or unsubscribe
   > <https://github.com/notifications/unsubscribe-auth/AAOZLZ5KHRD77UBI3NNU4S3TFMVPRANCNFSM4Y2SOMLA>
   > .
   >
   


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
poorejc commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-806903564


   @UncleGedd you've confirmed some of my suspicions that the HTTP request isn't going to work triggered against events at the tale end of the page lifecycle. 
   
   Regarding your thoughts:
   
   1. Sounds like an interesting idea. Will wait for more thoughts from @confusingstraw 
   
   2. We actually did have some user interest in a Websocket option for sendLogs. It's not a bad option for sendLogs, but I don't think it should be the 'only' option, which is what we're saying if we tie it to expected behavior for sendOnClose. The thing I don't like about that idea for canonical use is that it ties the library too closely to a specific backend. Our loose integration with back-end datastores was a design choice. I know some of our users are now using UserALE with different back-ends (not ELK).
   
   > Ok, so I've tried writing a cypress/journey test for `sendBeacon` and it's pretty flaky, but manual testing works in the browser (Chrome) consistently though
   
   +1. Same here.
   
   > RE 3. Safari doesn’t support visibility change events as uniformly as Moz and Chrome.
   
   I think after some more testing, we might end up settling on a conditional that sniffs for visibilitychange events and/or whatever most reliable equivalent is for Safari.
   


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc edited a comment on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
poorejc edited a comment on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-812802241


   Did some quick testing on https://github.com/apache/incubator-flagon-useralejs/tree/flagon-userale-50
   
   BLUF: flipped out visibilitychange for pagehide listener. Seeing both 'submit' and end of lifecyce events (click before page navigation) events as expected everytime using navigator.sendBeacon. Did about 2 sessions on Chrome and Safari each. Confirm visibilitychange handler will not work in safari. see below
   
   ```
   export function sendOnClose(logs, config) {
   window.addEventListener('pagehide', function () {
         navigator.sendBeacon(config.url, JSON.stringify(logs));
         logs.splice(0); // Clear array reference (no reassignment)
     });
   }
   ```
   
   Looking at @UncleGedd's unit test for https://github.com/apache/incubator-flagon-useralejs/tree/flagon-userale-50, its a test to see if the log queue is flushed when a given event fires. If I can get the test to work on pagehide, we might have a working unit test, skirting the difficulty to mock navigation.sendBeacon. I'll investigate.


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] UncleGedd commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
UncleGedd commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-808895462


   I spiked on the web worker option today and had good results. I listened for the `visibilitychange` event to fire `postMessage`, and the web worker was able to consistently perform the HTTP requests before the document fully closed. Understood that this may not work on all browsers and will require more testing. However, I think this is a clean approach that doesn't require anything extra on the backend (and optimizes a bit on the client side).
   
   Regarding websockets, I'm not sure where we would write the websocket server. It looks like the ELK stack only has support for [clients](https://www.elastic.co/guide/en/logstash/current/plugins-inputs-websocket.html)


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
poorejc commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-802497782


   Updated sendOnClose to flush logs queue:
   
   ```
   export function sendOnClose(logs, config) {
     document.addEventListener('visibilitychange', function () {
       if (document.visibilityState === 'hidden' && logs.length > 0) {
         navigator.sendBeacon(config.url, JSON.stringify(logs));
         logs.splice(0); // Clear array reference (no reassignment)
     }
   });
   ```
   resolves issue that any visibility change to 'hidden' state will send duplicate logs (e.g., switching tabs). This is desired behavior/intent for this function.


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
poorejc commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-808621268


   @confusingstraw you've always been someone I consider imminently sane.
   
   Sounds like the right way to go is [3] Websockets @confusingstraw makes a compelling case.
   - it solves the connectivity and event handling problems
   - it's something that's pretty ubiquitous
   - it aligns with previous user interest
   - the consequences of the "false positive" is that everything in the log queue is sent to back-end and queue is flushed. That results in neither duplicate logs or dropped logs. Seems like a low severity risk
   - the big drawback is that we have to manage/maintain supporting plugins for ELK and probably provide some other examples for plugins to a few other datastores (e.g., postgress). That's not too crazy.
   
   [1,2,4,5] seem like reaches and [5] seems like it would introduce other headaches. 
   
   If nothing else heard (feel free to add opinion here @kevbrowndev, @UncleGedd), I can put together some new tickets to cover this. Could use some help on this one.
   
   Other thing would like opinions on:
   
   To do this right, we should also make sure to mod and test our ELK stack and run full end-to-end integration tests. That could take a little longer. Is it worth doing a little more testing on this branch as is, pushing into next minor release (2.2.0) as a 'hot-fix', then doing the refactoring on sendLogs to websockets as the following release (2.3.0). Would be nice to make sure than NPM users have something incrementally better than current sendOnClose behavior (which doesn't behave at all?) Opinions?
   
   


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] confusingstraw commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
confusingstraw commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-802893470


   might be some interesting literature:
   [https://volument.com/blog/sendbeacon-is-broken](https://volument.com/blog/sendbeacon-is-broken)
   they are aiming for a slightly different goal, but the data is useful. they also include what is presumably a solution to #65 with their `Blob` usage.


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
poorejc commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-821766828


   > All of the tests in the [merged branch](https://github.com/apache/incubator-flagon-useralejs/tree/UncleGedd-sendOnClose) are passing on my end, using Node version v14.8.0 and I don't have any global packages.
   > 
   > When I was testing the viability of web workers I was swapping between tabs and closing the tab that the example page was open in. Both of those actions resulted in a seeing a custom web worker log in the terminal where the example server was running. I'm confident that the log was coming from the web worker itself, not `sendOnRefresh `. I was doing something like this in [worker.js](https://github.com/apache/incubator-flagon-useralejs/blob/UncleGedd-sendOnClose/src/worker.js):
   > 
   > ```js
   > export function workerSendLog(e) {
   >     const {url, logs} = e.data
   >     fetch(url, {
   >         method: 'POST',
   >         headers: {
   >             'Content-Type': 'application/json'
   >         },
   >         body: JSON.stringify([{webworker: 'from web worker'}]) // custom web worker log
   >     })
   > }
   > ```
   > 
   > One thing to make sure of is to set the `data-interval` to some value >1 in `example/index.html` and ensure that there are logs in the queue before closing the page (ie. click a couple of times and then close the tab to ensure there are logs to be flushed by the web worker).
   > 
   > I saw when running the example app that `index.js` was not being loaded, looks like this was due to Express not using the base directory to serve files from? I pushed a small change to the example app in that [original branch](https://github.com/UncleGedd/incubator-flagon-useralejs/tree/spike-workers).
   > 
   > Check out this video I made showing the behavior I'm seeing: https://youtu.be/ri0AHBMai20
   
   A few findings:
   
   1. Unit and Journey tests are now passing for me on this branch. May have had some collision when I last tested, also looks like node-fetch was missing from package, which broke jsdom-worker. Adding in that and tests are passing.
   
   2. **Webworkers do appear to send logs during tab changes**. I was able to replicate your test with the web-worker. I consistently saw custom messages from the web worker flipping between tabs (jacked log interval to 5 from 1). *They are reliably shipping data to the right URL and they are reliably firing off of the visibility change event*. 
   
   3. **Webworkers aren't working reliably for end of page life cycle visibility events**. In that same test [2], web workers fired reliably for tabbing back and forth. However, they did not work for submit and navigation events. Similar to my earlier testing (reported above), I do see submit and click events for linked buttons sporadically--they are very rare and unpredictable. I tested under a wide range of constraints and I don't see a pattern. The common theme is that **webworkers don't seem to work reliably (or at all) when the page is unloading**. I confirmed this in devtools looking at xml request traffic.
   
   4. **webworkers don't seem to work with pagehide events**. I'll test this some more, but I didn't see any behavior (e.g., [2]) using pagehide events. If true, that limits webworkers' utility in Safari.
   
   I'll do a little more testing soon, but I'm leaning toward fixing this issue with navigator.sendBeacon in the next release. Call it a hotfix. We can keep testing webworkers or move to websockets for more elegant solutions. I'll work with and look into your unit test for navigator.sendBeacon @UncleGedd that's very helpful, thanks!


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
poorejc commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-806930053


   > No problem! Also, welcome @kevbrowndev!
   > 
   > Regarding the other failing unit test:
   > 
   > ```js
   > it('sends logs on page exit without navigator', () => {
   >   // ...
   > }
   > ```
   > 
   > Is this unit test necessary? When will we ever not have the `navigator`?
   
   That's obsolete given that the HTTP option is flat broken. That will be removed/refactored once we find a Journey test that works for sendBeacon (if)


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc commented on issue #50: Last Log Drops from Log Queue when exiting page | specific configs

Posted by GitBox <gi...@apache.org>.
poorejc commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-801293796


   I have pushed the code that does work (commenting out the original code). However, it creates 2 other issues (at least):
   
   -breaks submit events. Submit events trigger a refresh. We use sendLogs (sendOnRefresh) to push those logs immediately when the event is detected. However, during the refresh cycle, the page enters into a hidden state. Result is we sometimes get duplicate logs (we're firing the logs off x2), but easy fix (remove sendOnRefresh) and keep testing.
   
   -breaks our logging server whenever page refreshes or navigate away or close--eg anytime sendOnClose would ship. Can see logs in ELK. Don't know why this happens.
   
   TypeError: req.body.forEach is not a function
       at /Users/jpoore/Documents/Apache_Flagon/test/incubator-flagon-useralejs/example/server.js:77:12
       at Layer.handle [as handle_request] (/Users/jpoore/Documents/Apache_Flagon/test/incubator-flagon-useralejs/node_modules/express/lib/router/layer.js:95:5)
       at next (/Users/jpoore/Documents/Apache_Flagon/test/incubator-flagon-useralejs/node_modules/express/lib/router/route.js:137:13)
       at Route.dispatch (/Users/jpoore/Documents/Apache_Flagon/test/incubator-flagon-useralejs/node_modules/express/lib/router/route.js:112:3)
       at Layer.handle [as handle_request] (/Users/jpoore/Documents/Apache_Flagon/test/incubator-flagon-useralejs/node_modules/express/lib/router/layer.js:95:5)
       at /Users/jpoore/Documents/Apache_Flagon/test/incubator-flagon-useralejs/node_modules/express/lib/router/index.js:281:22
       at Function.process_params (/Users/jpoore/Documents/Apache_Flagon/test/incubator-flagon-useralejs/node_modules/express/lib/router/index.js:335:12)
       at next (/Users/jpoore/Documents/Apache_Flagon/test/incubator-flagon-useralejs/node_modules/express/lib/router/index.js:275:10)
       at jsonParser (/Users/jpoore/Documents/Apache_Flagon/test/incubator-flagon-useralejs/node_modules/body-parser/lib/types/json.js:119:7)
       at Layer.handle [as handle_request] (/Users/jpoore/Documents/Apache_Flagon/test/incubator-flagon-useralejs/node_modules/express/lib/router/layer.js:95:5)
   
   
   
   Also, breaks 2 unit tests (expected -- will eventually need be fixed):   sendLogs
       ✓ sends logs on an interval
       ✓ does not send logs if the config is off
       1) sends logs on page exit with navigator
       2) sends logs on page exit without navigator
       ✓ does not send logs on page exit if config is off
   
   


----------------------------------------------------------------
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc commented on issue #50: Last Log Drops from Log Queue when exiting page | specific configs

Posted by GitBox <gi...@apache.org>.
poorejc commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-801278588


   document.addEventListener('visibilitychange', function () {
     if (document.visibilityState === 'hidden' && logs.length > 0) {
       sendLogs(logs, config, 1);
     }
   });
   
   This code does not work. sendLogs doesn't seem to work when the page is in this state.


----------------------------------------------------------------
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
poorejc commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-806908863


   > Ok, so I've tried writing a cypress/journey test for `sendBeacon` and it's pretty flaky, but manual testing works in the browser (Chrome) consistently though
   
   I'll also add that I'd rather a flaky unit/journey test than actual flaky behavior in prod, or in integration tests. I don't want to make the limitations of the test harness make us choose something less reliable in production. 
   
   We should include a fix for this (even if not 100% reliable) in next release. I'll keep digging on my end for examples for how to test against sendBeacon. It's been around a while and pretty widely used, so there has to be some fix/hack that someone has done for unit/journey tests.
   
   Thanks again @UncleGedd for fresh eyes on this.


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] UncleGedd commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
UncleGedd commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-812860922


   Thanks for the thorough testing! When I pushed up the branch all tests were passing so it may have been some inconsistency in our environments that was causing failing tests on your end. It is certainly strange that the web worker's network request appears in the dev tools but not in ELK. I'm able to consistently get the expected logs sent to the example app, but I haven't tried with a local ELK. 
   
   Nevertheless, I'm certainly not married to the web worker solution, sounds like it's pretty brittle. I may try another spike using service workers. Like Rob said above, they are supposed to exist regardless of what page is open.
   
   Let me know if I can be of any assistance on the `sendBeacon` solution. That sounds like the best way forward.


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
poorejc commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-815408726


   Great info, @UncleGedd. I’ll try testing it again with your parameters ASAP. Thanks again!
   
   > On Apr 6, 2021, at 4:45 PM, UncleGedd ***@***.***> wrote:
   > 
   > 
   > Also, you mentioned a sendNavigator unit test, will this suffice?
   > 
   >     it('sends logs on page exit with navigator', () => {
   >         const html = `<html><head></head><body></body></html>`;
   >         new JSDOM(html)
   >         const originalNavigator = global.navigator;
   >         let called = false;
   >         global.navigator = {
   >             sendBeacon: () => {
   >                 called = true;
   >             },
   >         };
   > 
   >         sendOnClose([{foo: 'bar'}], {on: true, url: 'test'});
   >         Object.defineProperty(global.document, 'visibilityState', {
   >             get: () => 'hidden'
   >         })
   >         global.document.dispatchEvent(new Event('visibilitychange'))
   > 
   >         global.navigator = originalNavigator
   >         expect(called).to.equal(true);
   >     });
   > —
   > You are receiving this because you were mentioned.
   > Reply to this email directly, view it on GitHub <https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-814427806>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AC474BSXS6OT4IWCHOHBMDDTHNXFZANCNFSM4Y2SOMLA>.
   > 
   
   


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] confusingstraw commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
confusingstraw commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-807044442


   @poorejc @UncleGedd 
   
   I will try to keep this sane.
   
   1. I don't think Web Workers help us very much, as [the spec](https://www.w3.org/TR/workers/) says that their behavior after their associated documents have gone away is implementation defined. Moreover, they are not allowed to respond to any additional events, so I don't believe they'd actually be able to observe the fact the page had closed. I could be wrong on the second point. Overall, there may be something there, but I am not confident that is the case.
   1. [Service Workers](https://developer.mozilla.org/en-US/docs/Web/API/Service_Worker_API) might be an effective alternative, as they let us run code regardless of whether the page is open (and regardless of whether there is an internet connection). The (large) downside is that it requires user-approval to install such a worker.
   1. Websockets are great, the main drawback there is that we need to keep one open. This is maybe not so bad, but we also need to make sure it stays connected during periods of poor connectivity. We'd actually still run into the same issue we have now, though, since we'd still need to send a message when the page closes. I do not see any great resources describing how well that works. We could, however, potentially sidestep that requirement altogether by instead letting the _server_ decide when the page was closed. By observing the client disconnecting from the server (and potentially not reconnecting for some time), we can assume they closed the page. This, however, introduces false positives if the client simply lost connection for a long period of time. Maybe not a bad thing, since we could replace "closed the page" with "disconnected".
   1. Another option would be keeping long-polling request or similar mechanic alive, and having the server assume that when it disconnects that the client has left the page. This has a similar drawback as the above.
   1. Lastly, I believe one of the articles I linked mentioned using `Image` objects as a form of data exfiltration. This could (maybe) be more reliable, but it would require the server to accept data in a different format (since it would arrive as part of the `src` URL).
   
   Hopefully this was all coherent. I think Websockets are maybe the best, but I am not confident they actually completely solve the problem.
   


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc removed a comment on issue #50: Last Log Drops from Log Queue when exiting page | specific configs

Posted by GitBox <gi...@apache.org>.
poorejc removed a comment on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-801278588


   document.addEventListener('visibilitychange', function () {
     if (document.visibilityState === 'hidden' && logs.length > 0) {
       sendLogs(logs, config, 1);
     }
   });
   
   This code does not work. sendLogs doesn't seem to work when the page is in this state.


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] UncleGedd edited a comment on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
UncleGedd edited a comment on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-806951761


   Gotcha. I was able to get this journey test passing consistently, we can also add a "retry" option to cypress where it'll attempt the test a couple times if it fails.
   
   ```js
       it('sends a log whenever a page is navigated away from', () => {
           cy.visit('http://localhost:8000')
           const spies = {beaconWasCalled: false};
           cy.window().then(win => {
               cy.stub(win.navigator, "sendBeacon", () => spies.beaconWasCalled = true);
               cy.contains('Link to Flagon Page').click()
           });
           cy.wrap(spies).its("beaconWasCalled").should("eq", true);
       });
   ```
   
   Pardon the homemade spy, I got the code from [here](https://gitter.im/cypress-io/cypress?at=5c4b838d454aad4df7a1218d), and it's the only thing I could get working consistently


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
poorejc commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-806171078


   I’d like to explore that RE alternatives to navigator.sendbeacon. Unfortunately, I can’t get sendLogs working against page unload event handlers (or visibility change events). The only thing that (I’ve tried) that works for this is sending data via navigator.sendbeacon triggered off of a visibility change event. So the ask wasn’t necessarily just to build unit or journey test, but really help shake this solution (integration test).
   
   The problems with this solutionare:
   
   1. navigator.sendbeacon isn’t as reliable as docs suggest (see Robs links in ticket)
   
   2. That method is difficult to programmatically test (unit and journey)
   
   3. Safari doesn’t support visibility change events as uniformly as Moz and Chrome.
   
   Need some more hands on testing before we commit to the current course of action.
   
   Joshua Poore
   
   
   > On Mar 24, 2021, at 12:56 PM, UncleGedd ***@***.***> wrote:
   > 
   > 
   > Ok, I've got one of the tests working:
   > 
   >     it('sends logs on page exit with navigator', () => {
   >         const html = `<html><head></head><body></body></html>`;
   >         new JSDOM(html)
   >         const originalNavigator = global.navigator;
   >         let called = false;
   >         global.navigator = {
   >             sendBeacon: () => {
   >                 called = true;
   >             },
   >         };
   > 
   >         sendOnClose([{foo: 'bar'}], {on: true, url: 'test'});
   >         Object.defineProperty(global.document, 'visibilityState', {
   >             get: () => 'hidden'
   >         })
   >         global.document.dispatchEvent(new Event('visibilitychange'))
   > 
   >         global.navigator = originalNavigator
   >         expect(called).to.equal(true);
   >     });
   > Basically I'm just faking the visibilitychange event and manually setting the visibilityState to "hidden" so the sendBeacon function will fire.
   > 
   > Regarding the other failing test:
   > 
   > it('sends logs on page exit without navigator', () => {
   >   // ...
   > }
   > In what case would we not have the navigator? Do we want to move away from using navigator.sendBeacon as the link Rob posted is suggesting?
   > 
   > —
   > You are receiving this because you were assigned.
   > Reply to this email directly, view it on GitHub, or unsubscribe.
   


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc closed issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
poorejc closed issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50


   


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc commented on issue #50: Last Log Drops from Log Queue when exiting page | specific configs

Posted by GitBox <gi...@apache.org>.
poorejc commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-801272089


   Regarding what methods to use instead: 
   
   https://developers.google.com/web/updates/2018/07/page-lifecycle-api and https://developer.mozilla.org/en-US/docs/Web/API/Navigator/sendBeacon (see Avoid unload and beforeunload). This appears to indicate that our sendOnClose function is using obsolete methods in modern browsers that have been optimized for cross-platform (desktop and mobile). 
   
   Both mozilla and chrome advocate for using visibilitychange events and detecting when the page's visible state is hidden.
   
   This method seems to capture the intent of sendOnClose, which is to dump logs when users are ending their session with a page/app.


----------------------------------------------------------------
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.

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



[GitHub] [incubator-flagon-useralejs] UncleGedd commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
UncleGedd commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-808896063


   @confusingstraw regarding service workers as an alternative in the comment above. Could you explain a bit more about how they require user-approval to install?


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] UncleGedd commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
UncleGedd commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-806919414


   No problem! Also, welcome @kevbrowndev! 
   
   Regarding the other failing unit test:
   ```js
   it('sends logs on page exit without navigator', () => {
     // ...
   }
   ```
   Is this unit test necessary? When will we ever not have the `navigator`?


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc edited a comment on issue #50: Last Log Drops from Log Queue when exiting page | specific configs

Posted by GitBox <gi...@apache.org>.
poorejc edited a comment on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-801190811


   Replicated bug: last click on page before redirect (linked button element) is not flushed out of queue to logging endpoint.
   
   desired behavior is that any dump of page triggers a flush of the log queue to the server. 
   
   Built example mock at ./example/index.html on https://github.com/apache/incubator-flagon-useralejs/tree/flagon-userale-50
   
   Confirmed with both example server and running single node ELK stack as logging backend. 
   
   NOTE: I'm seeing this issue for default params in .options. Also seeing this behavior when I kill the tab and when I navigate away, manually or directed through a link. Desired behavior should be the same for default or modified--dump of page should trigger sendOnClose function.


----------------------------------------------------------------
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.

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



[GitHub] [incubator-flagon-useralejs] UncleGedd commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
UncleGedd commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-806008674


   @confusingstraw, good call. That homemade spy has been there since before my time but I'll see if I can refactor it using sinon. (I think there's a few of these homemade spies floating around)


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] UncleGedd edited a comment on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
UncleGedd edited a comment on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-814359721


   All of the tests in the [merged branch](https://github.com/apache/incubator-flagon-useralejs/tree/UncleGedd-sendOnClose) are passing on my end, using Node version v14.8.0 and I don't have any global packages.
   
   When I was testing the viability of web workers I was swapping between tabs and closing the tab that the example page was open in. Both of those actions resulted in a seeing a custom web worker log in the terminal where the example server was running. I'm confident that the log was coming from the web worker itself, not `sendOnRefresh `.  I was doing something like this in [worker.js](https://github.com/apache/incubator-flagon-useralejs/blob/UncleGedd-sendOnClose/src/worker.js):
   ```js
   export function workerSendLog(e) {
       const {url, logs} = e.data
       fetch(url, {
           method: 'POST',
           headers: {
               'Content-Type': 'application/json'
           },
           body: JSON.stringify([{webworker: 'from web worker'}]) // custom web worker log
       })
   }
   ```
   
   One thing to make sure of is to set the `data-interval` to some value >1 in `example/index.html` and ensure that there are logs in the queue before closing the page (ie. click a couple of times and then close the tab to ensure there are logs to be flushed by the web worker).
   
   I saw when running the example app that `index.js` was not being loaded, looks like this was due to Express not using the base directory to serve files from? I pushed a small change to the example app in that [original branch](https://github.com/UncleGedd/incubator-flagon-useralejs/tree/spike-workers).
   
   Check out this video I made showing the behavior I'm seeing: https://youtu.be/ri0AHBMai20


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
poorejc commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-813493982


   Web-workers seem like a viable approach on its face. if @confusingstraw sees that it’s working, maybe it has legs—testing is a consensus game. One thing that might be affecting things is if you have npm modules installed globally that aren’t getting integrated locally. Usually, package-lock solves that, but sometimes not (my experience). I did test a direct pull of your branch a few nights ago and saw the same issues with unit test I reported out in the merged version. 
   
   Could be global/local module conflicts with some of the new packages in jsdom. I build exclusively from package files and don’t maintain any global modules. Also, what version of node.js are you using? I might be a minor version or two behind ‘latest', but I am running 15.x in testing (though didn’t see any LIFECYCLE error codes). Regardless, I’ll update to latest npm and test again. Also, happy to test again on whatever version you are.
   
   When you run tests in the merged branch UncleGedd-sendOnClose, are you seeing test failures?
   
   In observing logs, which kinds of events were you seeing in your testing that gave you confidence in your solution? Maybe I wasn’t clear in my previous comments, but i noticed that .src/attachHandlers code for refresh events still includes a sendOnRefresh function I hacked together: https://github.com/UncleGedd/incubator-flagon-useralejs/blob/f508dd95b69d129f68350e3f818ba3f28ad3fe0c/src/attachHandlers.js#L149-L154 <https://github.com/UncleGedd/incubator-flagon-useralejs/blob/f508dd95b69d129f68350e3f818ba3f28ad3fe0c/src/attachHandlers.js#L149-L154> If you were relying on ’submit’ events as proof of life for web-worker sendOnClose behavior, sendOnRefresh might have been doing the work you thought sendOnClose was. You’ll notice I took out that function in the original flagon-userale-50 branch and in the new branch I pushed with your commits (UncleGedd-sendOnClose).
   
   Overall, my proposal is this: let’s keep testing web-workers until we have consensus in testing (tests (unit/integration). If that doesn’t work we can explore web-sockets. However, right now navigator.sendBeacon is reliably passing integration tests. If we can mock a working unit test for that, we can release that as a patch in the next version (2.2.0). It is a less-than-ideal method. However, it’s working now (issues are theoretical and not yet observable). Point is: what we have now in 2.1.1 doesn’t work at all. navigator.sendBeacon will be more reliable than that. So keep testing web-workers, polish and push navigator.sendBeacon so we can close the open bug now, push navigator.sendBeacon in next release (unless breakthrough in web-worker/web-socket). Then, roll in a more elegant solution (web-worker/web-socket) in future release (2.2.1/2.3.0). Thoughts on this fake-it-till-you-make it solution?
   
   > On Apr 3, 2021, at 8:48 AM, UncleGedd ***@***.***> wrote:
   > 
   > 
   > Thanks for the thorough testing! When I pushed up the branch all tests were passing so it may have been some inconsistency in our environments that was causing failing tests on your end. It is certainly strange that the web worker's network request appears in the dev tools but not in ELK. I'm able to consistently get the expected logs sent to the example app, but I haven't tried with a local ELK.
   > 
   > Nevertheless, I'm certainly not married to the web worker solution, sounds like it's pretty brittle. I may try another spike using service workers. Like Rob said above, they are supposed to exist regardless of what page is open.
   > 
   > Let me know if I can be of any assistance on the sendBeacon solution. That sounds like the best way forward.
   > 
   > —
   > You are receiving this because you were mentioned.
   > Reply to this email directly, view it on GitHub <https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-812860922>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AC474BTTCMEOAJKPG67XE7TTG4FAHANCNFSM4Y2SOMLA>.
   > 
   
   


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] confusingstraw commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
confusingstraw commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-806065875


   if that is the case then it is probably my fault. undo my madness.


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc edited a comment on issue #50: Last Log Drops from Log Queue when exiting page | specific configs

Posted by GitBox <gi...@apache.org>.
poorejc edited a comment on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-801276755


   this code does work to the desired behavior:
   
   ```
   document.addEventListener('visibilitychange', function () {
       if (document.visibilityState === 'hidden' && logs.length > 0) {
         navigator.sendBeacon(config.url, JSON.stringify(logs));
       }
   }); 
   ```
   
   below is JSON snip from log firing from "linked button" element, which also opens a new page.
   
       "useraleVersion": "2.1.1",
       "toolName": "Apache UserALE.js Example (Custom)",
       "sessionID": "session_1616003276152",
       "type": "click",
       "path": [
         "button#Linked Button",
         "a",
         "body",
         "html",
         "#document",
         "Window"
   
   This code does not work. sendLogs doesn't seem to work when the page is in this state.
   
   ```
   document.addEventListener('visibilitychange', function () {
     if (document.visibilityState === 'hidden' && logs.length > 0) {
       sendLogs(logs, config, 1);
     }
   });
   ```
   
   


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
poorejc commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-811511744


   @UncleGedd thanks again! I'll take a look at this branch tomorrow, test like hell. Also, I'll try out some of the other events Rob discovered aside from visibilitychange and see if I get more reliable performance across browsers.


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
poorejc commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-807023697


   > Gotcha. I was able to get this journey test passing consistently, we can also add a "retry" option to cypress where it'll attempt the test a couple times if it fails.
   > 
   > ```js
   >     it('sends a log whenever a page is navigated away from', () => {
   >         cy.visit('http://localhost:8000')
   >         const spies = {beaconWasCalled: false};
   >         cy.window().then(win => {
   >             cy.stub(win.navigator, "sendBeacon", () => spies.beaconWasCalled = true);
   >             cy.contains('Link to Flagon Page').click()
   >         });
   >         cy.wrap(spies).its("beaconWasCalled").should("eq", true);
   >     });
   > ```
   > 
   > Pardon the homemade spy, I got the code from [here](https://gitter.im/cypress-io/cypress?at=5c4b838d454aad4df7a1218d), and it's the only thing I could get working consistently
   
   Welcome to Open Source, @UncleGedd--everything is "homemade" by a coalition of the willing :)


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc edited a comment on issue #50: Last Log Drops from Log Queue when exiting page | specific configs

Posted by GitBox <gi...@apache.org>.
poorejc edited a comment on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-801276755


   document.addEventListener('visibilitychange', function () {
       if (document.visibilityState === 'hidden' && logs.length > 0) {
         navigator.sendBeacon(config.url, JSON.stringify(logs));
       }
     }); 
   
   this code does work to the desired behavior. 
   
   below is JSON snip from log firing from "linked button" element, which also opens a new page.
   
       "useraleVersion": "2.1.1",
       "toolName": "Apache UserALE.js Example (Custom)",
       "sessionID": "session_1616003276152",
       "type": "click",
       "path": [
         "button#Linked Button",
         "a",
         "body",
         "html",
         "#document",
         "Window"


----------------------------------------------------------------
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.

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



[GitHub] [incubator-flagon-useralejs] UncleGedd commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
UncleGedd commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-805767149


   Sure! I'll take a look today


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] UncleGedd edited a comment on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
UncleGedd edited a comment on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-806951761


   Gotcha. I was able to get this journey test passing consistently, we can also add a "retry" option to cypress where it'll attempt the test a couple times if it fails.
   
   ```js
       it('sends a log whenever a page is navigated away from', () => {
           cy.visit('http://localhost:8000')
           const spies = {beaconWasCalled: false};
           cy.window().then(win => {
               cy.stub(win.navigator, "sendBeacon", () => spies.beaconWasCalled = true);
               cy.contains('Link to Flagon Page').click()
           });
           cy.wrap(spies).its("beaconWasCalled").should("eq", true);
       });
   ```
   
   Pardon the homemade spy, I got the code from [here](https://gitter.im/cypress-io/cypress?at=5c4b838d454aad4df7a1218d)


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc edited a comment on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
poorejc edited a comment on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-812314024


   > I finished up the spike and put it in a branch [here](https://github.com/UncleGedd/incubator-flagon-useralejs/tree/spike-workers). I was able to get a decent unit test in, but not a journey test. Currently, cypress does not support intercepting requests from workers. This solution works well in Chrome but not in Safari due to the lack of `visibilitychange` event support, and I haven't tested in Edge or IE. If you all like this solution, I'll continue the spike and start testing for other browsers.
   
   I cloned your branch @UncleGedd. Doing some testing I'm getting really inconsistent behavior. Rebuilding everything, I'm not seeing that our filters are getting applied, or that custom logs are being generated. That's wonky, so I want to test again tomorrow. Not sure why that's happening, but it could be my env and I want to start clean tomorrow and test methodically with fresh clones and clear cache. I'll also pull your branch into useralejs repo (new branch).
   
   I noticed that your branch doesn't remove sendOnRefresh from sendlogs (or refresh events in /src/attachHandlers). That would send submit logs when the page was refreshed without using sendOnClose...
   
   Also, the new JSDOM test is failing for me. I get this error:
   
   ```
     1) sendLogs
          flushes the log queue on visibility change:
        TypeError: Failed to execute 'dispatchEvent' on 'EventTarget': parameter 1 is not of type 'Event'.
         at Object.exports.convert (node_modules/jsdom/lib/jsdom/living/generated/Event.js:22:9)
         at Document.dispatchEvent (node_modules/jsdom/lib/jsdom/living/generated/EventTarget.js:228:24)
         at Context.<anonymous> (test/sendLogs_spec.js:132:25)
         at processImmediate (node:internal/timers:463:21)
   ```
   Can't find much wrong with your constructor, and didnt find much out digging through related issues on SO.
   
   Anyway, if you have the time and you want to test more: check to see that your not getting mouseovers on our example page (means filters aren't applying well). We should see custom decorated logs coming through clicking the first button. Also, removing sendOnRefresh from attachHandlers and sendLogs, if you see Submit events with webworkers, that makes me confident that they are working. 


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] UncleGedd commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
UncleGedd commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-814359721


   All of the tests in the [merged branch](https://github.com/apache/incubator-flagon-useralejs/tree/UncleGedd-sendOnClose) are passing on my end, using Node version v14.8.0 and I don't have any global packages.
   
   When I was testing the viability of web workers I was swapping between tabs and closing the tab that the example page was open in. Both of those actions resulted in a seeing a custom web worker log in the terminal where the example server was running. I'm confident that the log was coming from the web worker itself, not `sendOnRefresh `.  I was doing something like this in [worker.js](https://github.com/apache/incubator-flagon-useralejs/blob/UncleGedd-sendOnClose/src/worker.js):
   ```js
   export function workerSendLog(e) {
       const {url, logs} = e.data
       fetch(url, {
           method: 'POST',
           headers: {
               'Content-Type': 'application/json'
           },
           body: JSON.stringify([{webworker: 'from web worker'}]) // custom web worker log
       })
   }
   ```
   
   One thing to make sure of is to set the `data-interval` to some value >1 in `example/index.html` and ensure that there are logs in the queue before closing the page (ie. click a couple of times and then close the tab to ensure there are logs to be flushed by the web worker).
   
   I saw when running the example app that `index.js` was not being loaded, looks like this was due to Express not using the base directory to serve files from? I pushed a small change to the example app in that [merged branch](https://github.com/apache/incubator-flagon-useralejs/tree/UncleGedd-sendOnClose).
   
   Check out this video I made showing the behavior I'm seeing: https://youtu.be/ri0AHBMai20


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
poorejc commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-812314024


   > I finished up the spike and put it in a branch [here](https://github.com/UncleGedd/incubator-flagon-useralejs/tree/spike-workers). I was able to get a decent unit test in, but not a journey test. Currently, cypress does not support intercepting requests from workers. This solution works well in Chrome but not in Safari due to the lack of `visibilitychange` event support, and I haven't tested in Edge or IE. If you all like this solution, I'll continue the spike and start testing for other browsers.
   
   I cloned your branch @UncleGedd. Doing some testing I'm getting really inconsistent behavior. Rebuilding everything, I'm not seeing that our filters are getting applied, or that custom logs are being generated. That's wonky, so I want to test again tomorrow. Not sure why that's happening, but it could be my env and I want to start clean tomorrow and test methodically with fresh clones and clear cache. I'll also pull your branch into useralejs repo (new branch).
   
   I noticed that your branch doesn't remove sendOnRefresh from sendlogs (or refresh events in /src/attachHandlers). That would send submit logs when the page was refreshed without using sendOnClose...
   
   Also, the new JSDOM test is failing for me. I get this error:
   
     1) sendLogs
          flushes the log queue on visibility change:
        TypeError: Failed to execute 'dispatchEvent' on 'EventTarget': parameter 1 is not of type 'Event'.
         at Object.exports.convert (node_modules/jsdom/lib/jsdom/living/generated/Event.js:22:9)
         at Document.dispatchEvent (node_modules/jsdom/lib/jsdom/living/generated/EventTarget.js:228:24)
         at Context.<anonymous> (test/sendLogs_spec.js:132:25)
         at processImmediate (node:internal/timers:463:21)
   
   Can't find much wrong with your constructor, and didnt find much out digging through related issues on SO.
   
   Anyway, if you have the time and you want to test more: check to see that your not getting mouseovers on our example page (means filters aren't applying well). We should see custom decorated logs coming through clicking the first button. Also, removing sendOnRefresh from attachHandlers and sendLogs, if you see Submit events with webworkers, that makes me confident that they are working. 


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] UncleGedd commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
UncleGedd commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-806553427


   I don't think we are going to be able to send an HTTP request in those page unload event handlers, see this [SO thread](https://stackoverflow.com/questions/6162188/javascript-browsers-window-close-send-an-ajax-request-or-run-a-script-on-win). I know the answerer mentions the `unload` and `beforeunload` events, but I'm sure the same reasoning applies to `visibilitychange`
   
   It is indeed hard to ignore the stats from Rob's [link](https://volument.com/blog/sendbeacon-is-broken) regarding `sendBeacon`. Regardless, I don't think there is a way to get around using the `visibilitychange` event, and there are [solutions](https://www.smashingmagazine.com/2018/07/logging-activity-web-beacon-api/) to achieve a similar effect in Safari (search for "safari" in that link). Other ideas I'd like to test/mull over:
   
   1. [Web Workers](https://developer.mozilla.org/en-US/docs/Web/API/Worker): basic idea is to spawn a web worker whenever userale starts and do a `postMessage` on the `visibilitychange` (or any other) event, no more `sendBeacon`. (There may also be a larger conversation around using web workers in Userale as opposed to sending HTTP requests in the main browser thread, but I'm pretty inexperienced with this sort of thing, maybe @confusingstraw knows more about this)
   
   2. [Websockets](https://developer.mozilla.org/en-US/docs/Web/API/WebSocket): establish a websocket connection and send messages on the appropriate event, requires an [elasticsearch plugin](https://www.elastic.co/guide/en/logstash/current/plugins-inputs-websocket.html)
   
   Both of these solutions are more complicated than just using `sendBeacon` though. Regarding @poorejc's comment above:
   
   1. `navigator.sendBeacon` may not be as reliable as expected, but it's simple to use
   2. I think any solution will be difficult to test, although `sendBeacon` wasn't so bad to unit test, I'll write a journey test around it today
   3. Agreed, but there may be a workaround as described in the link above


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
poorejc commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-812796025


   @UncleGedd, @confusingstraw I beat up the webworkers pretty good in Chrome and Safari. 
   
   BLUF: The web-worker solution does appear to work, but there's still some work to do. Solution seems to be working in the client, but they are very unreliable in successfully posting to the back end (ELK and example server). Details follow:
   
   - I did see 'submit' events (after removing sendOnRefresh from refresh events in ./src/attachHandlers), and I did see a 'click' event on a button linked to a web link (e.g., navigate away from page).
   - Its behavior is terribly unreliable. I only observed the above events x2 in about 25-50 sessions on the example page in Chrome and Safari. That being said, I did reliably see expected script behavior and network events in devtools. No error messages or info messages. But generally, no logs coming through to ELK or our example server (tried both)
   - I'm NO expert in webworkers. I suspect that the they may be behaving as expected in the client, but some feature of web workers are causing them to fail in posting data correctly to our backends. Seems like the backend is accepting one message sent by the webworkers, but no more. Unclear to me the conditions under which it accepts a message from a webworker. Tried restarting backends, but doesn't seem to reliably work.
   
   Other things:
   - I played around with a few different configs in @UncleGedd's sendOnClose function. I tried eliminating the condition on configs turning off. This worked in my navigator.sendBeacon event. See: https://github.com/apache/incubator-flagon-useralejs/blob/0ec3645b828ff1429ec70beba8165102180d443b/src/sendLogs.js#L60-L66
   - I did see about the same behavior in chrome and safari with pagehide instead of visibilitychange. pagehide isn't as good as visibilitychange, but behavior is more consistent across browsers it seems. Something to ponder later, once we figure out how to reliably post logs. see below:
   
   ```
   export function sendOnClose(logs, config) {
     if (!config.on) {
       return;
     }
     const worker = createWorker(workerSendLog)
     window.addEventListener('pagehide', function() {
         worker.postMessage({url: config.url, logs});
         logs.splice(0, logs.length)
     });
   }
   ```
   -visibilitychange triggerend sendOnClose unit test is still failing for me.
   
   Lots more testing to do on web workers. I'm going to do some testing with navigator.sendBeacon and pagehide in Safari tonight. At the moment, that is our most reliable solution. I'd like to see if there are any JSDOM examples of mocks for navigator.sendBeacon. @UncleGedd if you have any thoughts or want to do deeper testing with webworkers, feel free. Don't want to discourage you on this--they seem to be producing the right behavior in the client, but they're currently failing integration testing.
   
   FYSA: I merged test branch commits into UncleGedd's branch, took out sendOnRefresh (which was doing the work pushing 'submit' events, not sendOnClose), and pushed to a new branch here: https://github.com/apache/incubator-flagon-useralejs/blob/UncleGedd-sendOnClose/src/sendLogs.js. I also added a navigation button to the example page to test for submit events and page navigation behavior both.
   


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] confusingstraw edited a comment on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
confusingstraw edited a comment on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-808897163


   i might just be crazy. i could've sworn they required some form of user interaction to enable. in either case, though, it _is_ possible to disable them entirely (as i have done so in my version of firefox). you can see the option at `about:config` under `dom.serviceWorkers.enabled`. you can see any installed workers (if enabled) at `about:serviceworkers`. this means we'd need some kind of fallback for it.
   
   i guess the same argument is also true of webworkers, as some extensions (like [uMatrix](https://addons.mozilla.org/en-US/firefox/addon/umatrix/)) allow users to disable web workers entirely.


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] UncleGedd commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
UncleGedd commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-806668259


   Ok, so I've tried writing a cypress/journey test for `sendBeacon` and it's pretty flaky. It does work in the browser (Chrome) consistently though


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
poorejc commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-805452663


   @UncleGedd are you open to helping me test this out a bit? I'm digging into this one tomorrow night, but if you're into it, I would love some more testing on this solution or any thoughts you have on an alternative.
   
   hacky fix is running here: https://github.com/apache/incubator-flagon-useralejs/tree/flagon-userale-50. Code link to the patch: https://github.com/apache/incubator-flagon-useralejs/blob/0ec3645b828ff1429ec70beba8165102180d443b/src/sendLogs.js#L63-L66


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] confusingstraw commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
confusingstraw commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-808897163


   i might just be crazy. i could've sworn they required some form of user interaction to enable. in either case, though, it _is_ possible to disable them entirely (as i have done so in my version of firefox). you can see the option at `about:config` under `dom.serviceWorkers.enabled`. you can see any installed works (if enabled) at `about:serviceworkers`. this means we'd need some kind of fallback for it.
   
   i guess the same argument is also true of webworkers, as some extensions (like [uMatrix](https://addons.mozilla.org/en-US/firefox/addon/umatrix/)) allow users to disable web workers entirely.


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc edited a comment on issue #50: Last Log Drops from Log Queue when exiting page | specific configs

Posted by GitBox <gi...@apache.org>.
poorejc edited a comment on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-801293796


   I have pushed the code that does work (commenting out the original code). However, it creates 2 other issues (at least):
   
   -breaks submit events. Submit events trigger a refresh. We use sendLogs (sendOnRefresh) to push those logs immediately when the event is detected. However, during the refresh cycle, the page enters into a hidden state. Result is we sometimes get duplicate logs (we're firing the logs off x2), but easy fix (remove sendOnRefresh) and keep testing.
   
   -breaks our logging server whenever page refreshes or navigate away or close--eg anytime sendOnClose would ship. Can see logs in ELK. Don't know why this happens.
   
   ```
   TypeError: req.body.forEach is not a function
       at /Users/jpoore/Documents/Apache_Flagon/test/incubator-flagon-useralejs/example/server.js:77:12
       at Layer.handle [as handle_request] (/Users/jpoore/Documents/Apache_Flagon/test/incubator-flagon-useralejs/node_modules/express/lib/router/layer.js:95:5)
       at next (/Users/jpoore/Documents/Apache_Flagon/test/incubator-flagon-useralejs/node_modules/express/lib/router/route.js:137:13)
       at Route.dispatch (/Users/jpoore/Documents/Apache_Flagon/test/incubator-flagon-useralejs/node_modules/express/lib/router/route.js:112:3)
       at Layer.handle [as handle_request] (/Users/jpoore/Documents/Apache_Flagon/test/incubator-flagon-useralejs/node_modules/express/lib/router/layer.js:95:5)
       at /Users/jpoore/Documents/Apache_Flagon/test/incubator-flagon-useralejs/node_modules/express/lib/router/index.js:281:22
       at Function.process_params (/Users/jpoore/Documents/Apache_Flagon/test/incubator-flagon-useralejs/node_modules/express/lib/router/index.js:335:12)
       at next (/Users/jpoore/Documents/Apache_Flagon/test/incubator-flagon-useralejs/node_modules/express/lib/router/index.js:275:10)
       at jsonParser (/Users/jpoore/Documents/Apache_Flagon/test/incubator-flagon-useralejs/node_modules/body-parser/lib/types/json.js:119:7)
       at Layer.handle [as handle_request] (/Users/jpoore/Documents/Apache_Flagon/test/incubator-flagon-useralejs/node_modules/express/lib/router/layer.js:95:5)
   ```
   
   
   Also, breaks 2 unit tests (expected -- will eventually need be fixed):   sendLogs
       ✓ sends logs on an interval
       ✓ does not send logs if the config is off
       1) sends logs on page exit with navigator
       2) sends logs on page exit without navigator
       ✓ does not send logs on page exit if config is off
   
   


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] confusingstraw commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
confusingstraw commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-808767416


   like i said, not entirely sure, but the spec does say that, once the document is closed, workers shouldn't be able to respond to any more events of any kind (no network updates, no new messages, zilch). if the `postMessage` occurs before the document goes away, then sure, that may work. always worth a shot.


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
poorejc commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-803046568


   That's great literature, @confusingstraw. Thanks! I'll checkout firing on pageHide and test against page refresh and navigation, closure. Looks like support for visibility change isn't great on safari. And, no I didn't test on safari, yet. Moz and Chrome are performing reliably though.
   
   I will note that for the life me, I just can't get sendLogs to work with any event in the page close lifecycle--visibilitychange, unload, beforeUnload. .sendBeacon is the only thing I can get to ship packages when the page closes. The only way we were getting submit events (submits trigger a page reload), was to execute sendLogs directly from the submit event (refreshEvents) handler in ./src/attachHandlers.
   
   Anyway, I'll do so further testing on pageHide and safari before I do a PR to test--I'll want some further review before we merge to test (especially since this breaks unit tests and may be undetectable my JSDOM methods--tests were passing before).


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] UncleGedd commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
UncleGedd commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-806951761


   Gotcha. I was able to get this journey test passing consistently, we can also add a "retry" option to cypress where it'll attempt the test a couple times if it fails.
   
   ```js
       it('sends a log whenever a page is navigated away from', () => {
           cy.visit('http://localhost:8000')
           const spies = {beaconWasCalled: false};
           cy.window().then(win => {
               cy.stub(win.navigator, "sendBeacon", () => spies.beaconWasCalled = true);
               cy.contains('Link to Flagon Page').click()
           });
           cy.wrap(spies).its("beaconWasCalled").should("eq", true);
       });
   ```


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc edited a comment on issue #50: Last Log Drops from Log Queue when exiting page | specific configs

Posted by GitBox <gi...@apache.org>.
poorejc edited a comment on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-801276755


   this code does work to the desired behavior:
   
   ```
   document.addEventListener('visibilitychange', function () {
       if (document.visibilityState === 'hidden' && logs.length > 0) {
         navigator.sendBeacon(config.url, JSON.stringify(logs));
       }
   }); 
   ```
   
   below is JSON snip from log firing from "linked button" element, which also opens a new page.
   
       "useraleVersion": "2.1.1",
       "toolName": "Apache UserALE.js Example (Custom)",
       "sessionID": "session_1616003276152",
       "type": "click",
       "path": [
         "button#Linked Button",
         "a",
         "body",
         "html",
         "#document",
         "Window"


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] UncleGedd commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
UncleGedd commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-823323862


   Good to know and thanks again for the thorough testing. Agreed that `navigator.sendBeacon` is the way forward for now.


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] UncleGedd commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
UncleGedd commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-814427806


   Also, you mentioned a `sendNavigator` unit test, will this suffice?
   
   ```js    
       it('sends logs on page exit with navigator', () => {
           const html = `<html><head></head><body></body></html>`;
           new JSDOM(html)
           const originalNavigator = global.navigator;
           let called = false;
           global.navigator = {
               sendBeacon: () => {
                   called = true;
               },
           };
   
           sendOnClose([{foo: 'bar'}], {on: true, url: 'test'});
           Object.defineProperty(global.document, 'visibilityState', {
               get: () => 'hidden'
           })
           global.document.dispatchEvent(new Event('visibilitychange'))
   
           global.navigator = originalNavigator
           expect(called).to.equal(true);
       });
   ```


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] confusingstraw edited a comment on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
confusingstraw edited a comment on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-808767416


   like i said, not entirely sure, but the spec does say that, once the document is closed, workers shouldn't be able to respond to any more events of any kind (no network updates, no new messages, zilch). if the `postMessage` occurs before the document goes away, then sure, that may work. always worth a shot.
   
   wrt the socket option, the (client-side) queue doesn't have any purpose except for batching the logs up and sending them, so as not to waste bandwidth. there is no such issue on the server side. the queue would still need to exist on the client, however, since we'd need to be able to flush them during periods of intermittent connectivity.


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc commented on issue #50: Last Log Drops from Log Queue when exiting page | specific configs

Posted by GitBox <gi...@apache.org>.
poorejc commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-802165728


   @UncleGedd This issues is a veritable existence proof for Journey Testing... Unit tests were deceptive given methods they used for mocking DOM. Resulted in lost data.


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] UncleGedd commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
UncleGedd commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-805993603


   Ok, I've got one of the tests working:
   ```js
       it('sends logs on page exit with navigator', () => {
           const html = `<html><head></head><body></body></html>`;
           new JSDOM(html)
           const originalNavigator = global.navigator;
           let called = false;
           global.navigator = {
               sendBeacon: () => {
                   called = true;
               },
           };
   
           sendOnClose([{foo: 'bar'}], {on: true, url: 'test'});
           Object.defineProperty(global.document, 'visibilityState', {
               get: () => 'hidden'
           })
           global.document.dispatchEvent(new Event('visibilitychange'))
   
           global.navigator = originalNavigator
           expect(called).to.equal(true);
       });
   ```
   Basically I'm just faking the `visibilitychange` event and manually setting the `visibilityState` to "hidden" so the `sendBeacon` function will fire. 
   
   Regarding the other failing test:
   ```js
   it('sends logs on page exit without navigator', () => {
     // ...
   }
   ```
   
   In what case would we not have the `navigator`? Do we want to move away from using `navigator.sendBeacon` as the link Rob posted is suggesting?


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] UncleGedd edited a comment on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
UncleGedd edited a comment on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-805993603


   Ok, I've got one of the tests working:
   ```js
       it('sends logs on page exit with navigator', () => {
           const html = `<html><head></head><body></body></html>`;
           new JSDOM(html)
           const originalNavigator = global.navigator;
           let called = false;
           global.navigator = {
               sendBeacon: () => {
                   called = true;
               },
           };
   
           sendOnClose([{foo: 'bar'}], {on: true, url: 'test'});
           Object.defineProperty(global.document, 'visibilityState', {
               get: () => 'hidden'
           })
           global.document.dispatchEvent(new Event('visibilitychange'))
   
           global.navigator = originalNavigator
           expect(called).to.equal(true);
       });
   ```
   Basically I'm just faking the `visibilitychange` event and manually setting the `visibilityState` to "hidden" so the `sendBeacon` function will fire. 
   
   Regarding the other failing test:
   ```js
   it('sends logs on page exit without navigator', () => {
     // ...
   }
   ```
   
   In what case would we not have the `navigator`? Do we want to move away from using `navigator.sendBeacon` as the [link](https://volument.com/blog/sendbeacon-is-broken) Rob posted is suggesting?


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] UncleGedd commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
UncleGedd commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-810428686


   I finished up the spike and put it in a branch [here](https://github.com/UncleGedd/incubator-flagon-useralejs/tree/spike-workers). I was able to get a decent unit test in, but not a journey test. Currently, cypress does not support intercepting requests from workers. This solution works well in Chrome but not in Safari due to the lack of `visibilitychange` event support, and I haven't tested in Edge or IE. If you all like this solution, I'll continue the spike and start testing for other browsers.


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc commented on issue #50: Last Log Drops from Log Queue when exiting page | specific configs

Posted by GitBox <gi...@apache.org>.
poorejc commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-801190811


   Replicated bug: last click on page before redirect (linked button element) is not flushed out of queue to logging endpoint.
   
   desired behavior is that any dump of page triggers a flush of the log queue to the server. 
   
   Built example mock at ./example/index.html on https://github.com/apache/incubator-flagon-useralejs/tree/flagon-userale-50
   
   Confirmed with both example server and running single node ELK stack as logging backend. 
   
   NOTE: I'm seeing this issue for default params in .options. Desired behavior should be the same for default or modified--dump of page should trigger sendOnClose function.


----------------------------------------------------------------
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc closed issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
poorejc closed issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50


   


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
poorejc commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-812802241


   Did some quick testing on https://github.com/apache/incubator-flagon-useralejs/tree/flagon-userale-50
   
   BLUF: flipped out visibilitychange for pagehide listener. Seeing both 'submit' and end of lifecyce events (click before page navigation) events as expected everytime. Did about 2 sessions on Chrome and Safari each. Confirm visibilitychange handler will not work in safari.
   
   Looking at @UncleGedd's unit test for https://github.com/apache/incubator-flagon-useralejs/tree/flagon-userale-50, its a test to see if the log queue is flushed when a given event fires. If I can get the test to work on pagehide, we might have a working unit test, skirting the difficulty to mock navigation.sendBeacon.


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc commented on issue #50: Last Log Drops from Log Queue when exiting page | specific configs

Posted by GitBox <gi...@apache.org>.
poorejc commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-801294930


   NOTE: do extra testing on Browser Plugin. Could be non issue, could be big issue.


----------------------------------------------------------------
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc edited a comment on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
poorejc edited a comment on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-812796025


   @UncleGedd, @confusingstraw I beat up the webworkers pretty good in Chrome and Safari. 
   
   BLUF: The web-worker solution does appear to work, but there's still some work to do. Solution seems to be working in the client, but they are very unreliable in successfully posting to the back end (ELK and example server). Details follow:
   
   - I did see 'submit' events (after removing sendOnRefresh from refresh events in ./src/attachHandlers), and I did see a 'click' event on a button linked to a web link (e.g., navigate away from page).
   - Its behavior is terribly unreliable. I only observed the above events x2 in about 25-50 sessions on the example page in Chrome and Safari. That being said, I did reliably see expected script behavior and network events in devtools. No error messages or info messages. But generally, no logs coming through to ELK or our example server (tried both)
   - I'm NO expert in webworkers. I suspect that the they may be behaving as expected in the client, but some feature of web workers are causing them to fail in posting data correctly to our backends. Seems like the backend is accepting one message sent by the webworkers, but no more. Unclear to me the conditions under which it accepts a message from a webworker. Tried restarting backends, but doesn't seem to reliably work.
   
   Other things:
   - I played around with a few different configs in @UncleGedd's sendOnClose function. I tried eliminating the condition on configs turning off. This worked in my navigator.sendBeacon event. See: https://github.com/apache/incubator-flagon-useralejs/blob/0ec3645b828ff1429ec70beba8165102180d443b/src/sendLogs.js#L60-L66
   - I did see about the same behavior in chrome and safari with pagehide instead of visibilitychange. pagehide isn't as good as visibilitychange, but behavior is more consistent across browsers it seems. Something to ponder later, once we figure out how to reliably post logs. see below:
   
   ```
   export function sendOnClose(logs, config) {
     if (!config.on) {
       return;
     }
     const worker = createWorker(workerSendLog)
     window.addEventListener('pagehide', function() {
         worker.postMessage({url: config.url, logs});
         logs.splice(0, logs.length)
     });
   }
   ```
   -visibilitychange triggerend sendOnClose unit test is still failing for me.
   
   Lots more testing to do on web workers. I'm going to do some testing with navigator.sendBeacon and pagehide in Safari tonight. At the moment, that is our most reliable solution. I'd like to see if there are any JSDOM examples of mocks for navigator.sendBeacon. @UncleGedd if you have any thoughts or want to do deeper testing with webworkers, feel free. Don't want to discourage you on this--they seem to be producing the right behavior in the client, but they're currently failing integration testing.
   
   FYSA: I merged test branch commits into UncleGedd's branch, took out sendOnRefresh (which was doing the work pushing 'submit' events, not sendOnClose), and pushed to a new branch here: https://github.com/apache/incubator-flagon-useralejs/tree/UncleGedd-sendOnClose. I also added a navigation button to the example page to test for submit events and page navigation behavior both.
   


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] confusingstraw commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
confusingstraw commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-802896819


   another good one:
   [https://calendar.perfplanet.com/2020/beaconing-in-practice/](https://calendar.perfplanet.com/2020/beaconing-in-practice/)


-- 
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.

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



[GitHub] [incubator-flagon-useralejs] poorejc commented on issue #50: Last Log Drops from Log Queue when exiting page | specific configs

Posted by GitBox <gi...@apache.org>.
poorejc commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-801276755


   document.addEventListener('visibilitychange', function () {
       if (document.visibilityState === 'hidden' && logs.length > 0) {
         navigator.sendBeacon(config.url, JSON.stringify(logs));
       }
     }); 
   
   this code does work to the desired behavior. 


----------------------------------------------------------------
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.

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



[GitHub] [incubator-flagon-useralejs] confusingstraw commented on issue #50: sendOnClose function broken -- logs dropped when users navigate away from page

Posted by GitBox <gi...@apache.org>.
confusingstraw commented on issue #50:
URL: https://github.com/apache/incubator-flagon-useralejs/issues/50#issuecomment-808895826


   hey, if it works, it works. i don't think there is a need for websockets if the webworker fix seems to do the trick. they'd be a nice addition, but no need to pile on work that doesn't actually bring a lot to the table. i'd imagine we want to move all of the queuing/`sendLog` work into the worker.


-- 
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.

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