[BUG] App insight library will flush telemetry using beforeUnload event but this event is cancellable

[thoo1]

Description/Screenshot

• appinsight sdk will flush telemetry on beforeUnload event.
  

  ApplicationInsights-JS/AISKU/src/Initialization.ts

  Line 487 in ef5323d

  let added = addEventHandler('beforeunload', performHousekeeping);

• however this event is cancellable, for example, our app asks if user really wants to navigate away from page
• user may cancel the event, which means the page did not unload.
• why this is a problem
  • we have a use case to mark all the logs flowing to app insight while the page is actually unloading with a flag "IsPageUnloading"
  • we plan to use the pagehide event to set this flag to true and put it in every telemetry log using an interceptor
  • since the logs are being flushed in beforeUnload, we cannot set this flag to true since pagehide event occurs after beforeUnload event. And the difference in time between these two events is up to the user (the user could take a long time to click confirm/cancel the page navigation)

It looks like app insight sdk is also adding the same event handler to the pagehide event, so I'm wondering if having the event handler for the beforeUnload event is necessary

Steps to Reproduce

• OS/Browser: Windows/Chrome
• SDK Version [e.g. 22]: 2.4.0
• How you initialized the SDK:

    this.appInsights = new ApplicationInsights({
      config: {
        instrumentationKey: Environment.config.appInsightsInstrumentationKey,
        endpointUrl: Environment.config.appInsightsEndpointUrl,
        extensions: [this.reactPlugin],
        extensionConfig: {
          [this.reactPlugin.identifier]: { history }
        },
        disableFetchTracking: false,
        disableAjaxTracking: false,
        enableCorsCorrelation: true,
        disableCorrelationHeaders: false,
        enableAutoRouteTracking: true,
        maxAjaxCallsPerView: -1,
        enableResponseHeaderTracking: true
      }
    });

Expected behavior

• that appinsights only flushes the logs when page is actually unloading, as beforeUnload event is cancellable

Additional context
Add any other context about the problem here.

[MSNev]

Historically, having both beforeunload and pagehide is required to ensure that we can flush events in all environments, as not all of the supported runtimes (browser, mobile devices, etc) you have however, identified an issue in that we should also be listening to the "unload" event... Will raise a new bug for that.

At what point are you tagging the events with "IsPageUnloading"?
The flush operation is simply "sending" the cached events (which have already been converted to a string version of JSON), so there are no telemetry initializers that would be run for these "cached" events.... Additionally, unless your runtime doesn't support sendBeacon and/or you set the onunloadDisableBeacon the events will actually be sent using sendBeacon().

If you want to control the level for when the events are flushed, the workaround I would suggest is as follows, as while we will add the unload event we won't be removing before unload due to potential backward compatibility issues.

• In your config set the "disableFlushOnBeforeUnload" to true (this will de-activate the above logic)
• Listen to the event(s) that you want to use to determine the navigate away simply call this.appInsights.onunloadFlush(false); (which is what these events are doing)

[MSNev]

As mentioned above raised new bug to more correctly track all unload events in all environments #1517

[thoo1]

thanks for the suggestion @MSNev, it makes sense, I'll try out what you said, to let our app control when we want app insights to unload.

At what point are you tagging the events with "IsPageUnloading"?

We were previously using beforeunload event to tag the events with IsPageUnloading but now that we also understand that this is cancellable, we are trying to move away from that event and use pagehide event (after beforeunload but before unload)

[thoo1]

@MSNev related bug, I found that adding disableFlushOnBeforeUnload: true in my config does not result in skipping this line

ApplicationInsights-JS/AISKU/src/Initialization.ts

Line 484 in ef5323d

if (!appInsightsInstance.appInsights.config.disableFlushOnBeforeUnload) {

appInsightsInstance.appInsights.config.disableFlushOnBeforeUnload is still undefined. I can however find the config that I want to pass in appInsightsInstance.config.disableFlushOnBeforeUnload

I found that in the creation of appInsightsInstance.appInsights, the constructor is not passed the config, not sure if it should or not

ApplicationInsights-JS/AISKU/src/Initialization.ts

Line 130 in ef5323d

_self.appInsights = new ApplicationInsights();

Config:

    this.appInsights = new ApplicationInsights({
      config: {
        instrumentationKey: Environment.config.appInsightsInstrumentationKey,
        endpointUrl: Environment.config.appInsightsEndpointUrl,
        extensions: [this.reactPlugin],
        extensionConfig: {
          [this.reactPlugin.identifier]: { history }
        },
        disableFetchTracking: false,
        disableAjaxTracking: false,
        enableCorsCorrelation: true,
        disableCorrelationHeaders: false,
        enableAutoRouteTracking: true,
        maxAjaxCallsPerView: -1,
        enableResponseHeaderTracking: true,
        disableFlushOnBeforeUnload: true,
      }
    });

[EduardBoginya]

I got the same issue with disableFlushOnBeforeUnload: true that @thoo1 mentioned, passing that prop to config does not affect onunload behavior

[MSNev]

Investigating the disableFlushOnBeforeUnload issue

[MSNev]

Ok, I've identified the issue with the config. Simplistically, depending on the version of the snippet that is used this initializes different base objects.
When using

• When using a current snippet it initializes the extension/ai-analytics-js/ApplicationInsights instance as appInsightsInstance.appInsights which is NOT initializing the disable* config items... (This is part of the bug).
• But when using the legacy snippet this initializes the ApplicationInsightsDeprecated instance as appInsightsInstance.appInsights which initializes the disable* config items and the extension/ai-analytics-js/ApplicationInsights instance is linked as appInsightsInstance.appInsightsNew as this is where most of the work is done.

This looks like it's been broken like this for a while (so not going to elevate this to a P1), but linking to #1517 as there is a bunch of code that needs fixing here to restore compatibility.

Linking #1517 as there is a bunch of code that is required to be fixed, also created a new bug to explicitly call out the 2 disable config items #1524

[MSNev]

Tagging PR #1532

[MSNev]

v2.6.2 is now fully deployed

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.