Instrument timed out and cancelled redirects (#7989)

- Instrument timed out and cancelled redirects

Author: konstantin-msft

change/@azure-msal-browser-b154fb66-b5b9-45de-ac7e-9425a8ea389c.json

@@ -0,0 +1,7 @@
+{
+  "type": "minor",
+  "comment": "Instrument timed out and cancelled redirects #7989",
+  "packageName": "@azure/msal-browser",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

change/@azure-msal-common-3b862126-d7a7-4829-b6fd-5711a5c2a8b3.json

@@ -0,0 +1,7 @@
+{
+  "type": "minor",
+  "comment": "Instrument timed out and cancelled redirects #7989",
+  "packageName": "@azure/msal-common",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

lib/msal-browser/src/controllers/StandardController.ts

@@ -641,11 +641,11 @@ export class StandardController implements IController {
                 typeof configOnRedirectNavigateCb === "function"
                     ? configOnRedirectNavigateCb(url)
                     : undefined;
-            if (navigate !== false) {
-                atrMeasurement.end({ success: true });
-            } else {
-                atrMeasurement.discard();
-            }
+            atrMeasurement.add({
+                navigateCallbackResult: navigate !== false,
+            });
+            atrMeasurement.event =
+                atrMeasurement.end({ success: true }) || atrMeasurement.event;
             return navigate;
         };
 
@@ -721,7 +721,21 @@ export class StandardController implements IController {
             return await result;
         } catch (e) {
             this.browserStorage.resetRequestCache();
-            atrMeasurement.end({ success: false }, e);
+            /*
+             * Pre-redirect event completes before navigation occurs.
+             * Timed out navigation needs to be instrumented separately as a post-redirect event.
+             */
+            if (atrMeasurement.event.status === 2) {
+                this.performanceClient
+                    .startMeasurement(
+                        BrowserRootPerformanceEvents.AcquireTokenRedirect,
+                        correlationId
+                    )
+                    .end({ success: false }, e);
+            } else {
+                atrMeasurement.end({ success: false }, e);
+            }
+
             if (isLoggedIn) {
                 this.eventHandler.emitEvent(
                     EventType.ACQUIRE_TOKEN_FAILURE,

lib/msal-browser/test/app/PublicClientApplication.spec.ts

@@ -2161,6 +2161,107 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
             });
         });
 
+        it("emits pre-redirect telemetry event when onRedirectNavigate callback is set in configuration", (done) => {
+            const onRedirectNavigate = (url: string) => {
+                expect(url).toBeDefined();
+            };
+
+            pca = new PublicClientApplication({
+                auth: {
+                    clientId: TEST_CONFIG.MSAL_CLIENT_ID,
+                    onRedirectNavigate,
+                },
+                telemetry: {
+                    client: new BrowserPerformanceClient(testAppConfig),
+                    application: {
+                        appName: TEST_CONFIG.applicationName,
+                        appVersion: TEST_CONFIG.applicationVersion,
+                    },
+                },
+            });
+            pca = (pca as any).controller;
+
+            pca.initialize().then(() => {
+                const callbackId = pca.addPerformanceCallback((events) => {
+                    expect(events[0].success).toBe(true);
+                    expect(events[0].name).toBe(
+                        BrowserRootPerformanceEvents.AcquireTokenPreRedirect
+                    );
+                    expect(events[0].navigateCallbackResult).toBeTruthy();
+                    pca.removePerformanceCallback(callbackId);
+                    done();
+                });
+
+                jest.spyOn(
+                    NavigationClient.prototype,
+                    "navigateExternal"
+                ).mockImplementation(() => Promise.resolve(true));
+
+                jest.spyOn(
+                    PkceGenerator,
+                    "generatePkceCodes"
+                ).mockResolvedValue({
+                    challenge: TEST_CONFIG.TEST_CHALLENGE,
+                    verifier: TEST_CONFIG.TEST_VERIFIER,
+                });
+                const loginRequest: RedirectRequest = {
+                    redirectUri: TEST_URIS.TEST_REDIR_URI,
+                    scopes: ["user.read", "openid", "profile"],
+                    state: TEST_STATE_VALUES.USER_STATE,
+                };
+                pca.acquireTokenRedirect(loginRequest);
+            });
+        });
+
+        it("instruments pre-redirect telemetry event when navigation times out", (done) => {
+            let eventCounter = 0;
+            const callbackId = pca.addPerformanceCallback((events) => {
+                if (
+                    events[0].name ===
+                    BrowserRootPerformanceEvents.AcquireTokenPreRedirect
+                ) {
+                    expect(events[0].success).toBe(true);
+                    eventCounter++;
+                }
+
+                if (
+                    events[0].name ===
+                    BrowserRootPerformanceEvents.AcquireTokenRedirect
+                ) {
+                    expect(events[0].success).toBe(false);
+                    expect(events[0].errorCode).toEqual("timed_out");
+                    eventCounter++;
+                }
+
+                if (eventCounter === 2) {
+                    pca.removePerformanceCallback(callbackId);
+                    done();
+                }
+            });
+
+            jest.spyOn(
+                NavigationClient.prototype,
+                "navigateExternal"
+            ).mockRejectedValue(
+                createBrowserAuthError(
+                    BrowserAuthErrorCodes.timedOut,
+                    "failed_to_redirect"
+                )
+            );
+
+            jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({
+                challenge: TEST_CONFIG.TEST_CHALLENGE,
+                verifier: TEST_CONFIG.TEST_VERIFIER,
+            });
+            const loginRequest: RedirectRequest = {
+                redirectUri: TEST_URIS.TEST_REDIR_URI,
+                scopes: ["user.read", "openid", "profile"],
+                state: TEST_STATE_VALUES.USER_STATE,
+            };
+
+            pca.acquireTokenRedirect(loginRequest).catch((e) => {});
+        });
+
         it("emits pre-redirect telemetry event when onRedirectNavigate callback is not set", (done) => {
             const callbackId = pca.addPerformanceCallback((events) => {
                 expect(events[0].success).toBe(true);
@@ -2234,7 +2335,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
             await pca.acquireTokenRedirect(loginRequest);
         });
 
-        it("discards pre-redirect telemetry event when onRedirectNavigate callback returns false", async () => {
+        it("discards pre-redirect telemetry event when onRedirectNavigate callback returns false", (done) => {
             const onRedirectNavigate = (url: string) => {
                 return false;
             };
@@ -2253,38 +2354,43 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
                 },
             });
             pca = (pca as any).controller;
-            await pca.initialize();
+            pca.initialize().then(() => {
+                const callbackId = pca.addPerformanceCallback((events) => {
+                    expect(events[0].success).toBe(true);
+                    expect(events[0].name).toBe(
+                        BrowserRootPerformanceEvents.AcquireTokenPreRedirect
+                    );
+                    expect(events[0].navigateCallbackResult).toBeFalsy();
+                    pca.removePerformanceCallback(callbackId);
+                    done();
+                });
 
-            const callbackId = pca.addPerformanceCallback((events) => {
-                expect(events[0].success).toBe(true);
-                expect(events[0].name).toBe(
-                    BrowserRootPerformanceEvents.AcquireTokenPreRedirect
+                const measurementDiscardSpy = jest.spyOn(
+                    PerformanceClient.prototype,
+                    "discardMeasurements"
                 );
-                pca.removePerformanceCallback(callbackId);
-            });
 
-            const measurementDiscardSpy = jest.spyOn(
-                PerformanceClient.prototype,
-                "discardMeasurements"
-            );
+                jest.spyOn(
+                    NavigationClient.prototype,
+                    "navigateExternal"
+                ).mockResolvedValue(true);
 
-            jest.spyOn(
-                NavigationClient.prototype,
-                "navigateExternal"
-            ).mockResolvedValue(true);
+                jest.spyOn(
+                    PkceGenerator,
+                    "generatePkceCodes"
+                ).mockResolvedValue({
+                    challenge: TEST_CONFIG.TEST_CHALLENGE,
+                    verifier: TEST_CONFIG.TEST_VERIFIER,
+                });
+                const loginRequest: RedirectRequest = {
+                    redirectUri: TEST_URIS.TEST_REDIR_URI,
+                    scopes: ["user.read", "openid", "profile"],
+                    state: TEST_STATE_VALUES.USER_STATE,
+                };
 
-            jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({
-                challenge: TEST_CONFIG.TEST_CHALLENGE,
-                verifier: TEST_CONFIG.TEST_VERIFIER,
+                pca.acquireTokenRedirect(loginRequest);
+                expect(measurementDiscardSpy).toHaveBeenCalledTimes(0);
             });
-            const loginRequest: RedirectRequest = {
-                redirectUri: TEST_URIS.TEST_REDIR_URI,
-                scopes: ["user.read", "openid", "profile"],
-                state: TEST_STATE_VALUES.USER_STATE,
-            };
-
-            await pca.acquireTokenRedirect(loginRequest);
-            expect(measurementDiscardSpy).toHaveBeenCalledTimes(1);
         });
 
         it("instruments initialization error", (done) => {

lib/msal-common/apiReview/msal-common.api.md

@@ -3406,6 +3406,7 @@ export type PerformanceEvent = {
     usePreGeneratedPkce?: boolean;
     msalInstanceCount?: number;
     sameClientIdInstanceCount?: number;
+    navigateCallbackResult?: boolean;
 };
 
 declare namespace PerformanceEvents {

lib/msal-common/src/telemetry/performance/PerformanceEvent.ts

@@ -329,6 +329,8 @@ export type PerformanceEvent = {
     msalInstanceCount?: number;
     // Number of MSAL JS instances using the same client id in the frame
     sameClientIdInstanceCount?: number;
+
+    navigateCallbackResult?: boolean;
 };
 
 export type PerformanceEventContext = {