Skip to content

Commit 378a885

Browse files
fix: address multi-model review findings in App Insights telemetry
- Remove disableCookiesUsage: true — was silently breaking cross-session anonymous user analytics (every tab = new user ID). Consented users now correctly persist ai_user across sessions. - onConsentRevoked() explicitly deletes ai_user/ai_session cookies on every page load, unconditionally (not guarded on appInsights init). Covers the critical case: returning visitor with stale cookies from a prior consented session who then denied consent and closed the browser. - Fix setAuthenticatedContext() no-op: call instance.setAuthenticatedUserContext() directly in createAppInsights() instead of via module-level guard which was always null at that point. - Reset sdkLoadPromise=null on onerror to allow retry on transient CDN failure. - Add 15s timeout in ensureSdkLoaded() when attaching to existing script tag to prevent hang if script already errored before listeners were attached. - Add ai_user/ai_session to consent-manager clearTrackingCookies() as defense-in-depth for the 'forget me' revocation path. Reviewed and approved by Opus 4.6 and GPT-5.5.
1 parent daebe95 commit 378a885

2 files changed

Lines changed: 41 additions & 10 deletions

File tree

EssentialCSharp.Web/wwwroot/js/appinsights-manager.js

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,18 @@
5959
sdkLoadPromise = new Promise((resolve, reject) => {
6060
const existing = document.querySelector(`script[src="${SDK_URL}"]`);
6161
if (existing) {
62-
existing.addEventListener("load", () => resolve(), { once: true });
63-
existing.addEventListener("error", () => reject(new Error("Failed to load App Insights SDK.")), { once: true });
62+
// Guard: script may have already loaded successfully
63+
if (window.Microsoft?.ApplicationInsights?.ApplicationInsights) {
64+
resolve();
65+
return;
66+
}
67+
// Guard: script may have already errored — add timeout so promise doesn't hang forever
68+
const timeoutId = setTimeout(() => {
69+
sdkLoadPromise = null;
70+
reject(new Error("App Insights SDK load timed out."));
71+
}, 15000);
72+
existing.addEventListener("load", () => { clearTimeout(timeoutId); resolve(); }, { once: true });
73+
existing.addEventListener("error", () => { clearTimeout(timeoutId); sdkLoadPromise = null; reject(new Error("Failed to load App Insights SDK.")); }, { once: true });
6474
return;
6575
}
6676

@@ -69,7 +79,10 @@
6979
script.async = true;
7080
script.defer = true;
7181
script.onload = () => resolve();
72-
script.onerror = () => reject(new Error("Failed to load App Insights SDK."));
82+
script.onerror = () => {
83+
sdkLoadPromise = null; // allow retry on transient failure
84+
reject(new Error("Failed to load App Insights SDK."));
85+
};
7386
document.head.appendChild(script);
7487
});
7588

@@ -94,7 +107,13 @@
94107
});
95108

96109
instance.loadAppInsights();
97-
setAuthenticatedContext();
110+
111+
// Set authenticated context on `instance` directly — the module-level `appInsights` variable
112+
// is not yet assigned at this point, so setAuthenticatedContext() would be a no-op.
113+
const userId = getAuthenticatedUserId();
114+
if (userId) {
115+
instance.setAuthenticatedUserContext(userId);
116+
}
98117

99118
if (!didInitialPageView) {
100119
instance.trackPageView();
@@ -126,12 +145,24 @@
126145
}
127146

128147
function onConsentRevoked() {
129-
if (!appInsights) {
130-
return;
148+
clearAuthenticatedContext(); // guards internally
149+
if (appInsights) {
150+
appInsights.config.disableTelemetry = true;
131151
}
132152

133-
clearAuthenticatedContext();
134-
appInsights.config.disableTelemetry = true;
153+
// Run unconditionally — appInsights may never have been initialized this session
154+
// (user has always denied), but ai_user/ai_session cookies from a prior consented
155+
// session can still be present in the browser.
156+
// consent-manager.clearTrackingCookies() only runs on the "forget me" path;
157+
// normal reject/revoke flows fire the consent event without calling it.
158+
const expired = "expires=Thu, 01 Jan 1970 00:00:00 GMT";
159+
const secure = window.location.protocol === "https:" ? ";Secure" : "";
160+
const hostname = window.location.hostname;
161+
["ai_user", "ai_session"].forEach(function (name) {
162+
document.cookie = `${name}=;${expired};path=/${secure}`;
163+
document.cookie = `${name}=;${expired};path=/;domain=${hostname}${secure}`;
164+
document.cookie = `${name}=;${expired};path=/;domain=.${hostname}${secure}`;
165+
});
135166
}
136167

137168
function syncConsentState() {

EssentialCSharp.Web/wwwroot/js/consent-manager.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -450,8 +450,8 @@ class ConsentManager {
450450
}
451451

452452
clearTrackingCookies() {
453-
// Clear common tracking cookies (Google Analytics and Microsoft Clarity)
454-
const trackingCookies = ['_ga', '_gid', '_gat', '_clck', '_clsk', 'CLID', 'ANONCHK', 'MR', 'MUID', 'SM'];
453+
// Clear common tracking cookies (Google Analytics, Microsoft Clarity, and App Insights)
454+
const trackingCookies = ['_ga', '_gid', '_gat', '_clck', '_clsk', 'CLID', 'ANONCHK', 'MR', 'MUID', 'SM', 'ai_user', 'ai_session'];
455455
const expired = 'expires=Thu, 01 Jan 1970 00:00:00 GMT';
456456
const hostname = window.location.hostname;
457457
// Build candidate domains: exact host plus progressively shorter parent domains.

0 commit comments

Comments
 (0)