Collect additional telemetry for crypto/runtime errors (#7394)

- Adds additional check for the subtle property on crypto
- Fixes error stack logic to keep top 5 lines instead of bottom 5
- Collects common, known runtime errors and redacts when necessary

---------

Co-authored-by: Konstantin Shabelko <[email protected]>

change/@azure-msal-browser-a7e3c096-c445-4017-9b15-e9c0a67e1aa4.json

@@ -0,0 +1,7 @@
+{
+  "type": "minor",
+  "comment": "Capture runtime errors in telemetry for submeasures",
+  "packageName": "@azure/msal-browser",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

change/@azure-msal-common-e50cbab0-daa5-497d-af15-caeed7c34e6c.json

@@ -0,0 +1,7 @@
+{
+  "type": "minor",
+  "comment": "Validate crypto.subtle is available during initialization",
+  "packageName": "@azure/msal-common",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

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

@@ -184,7 +184,7 @@ function blockReloadInHiddenIframes(): void;
 //
 // @public
 export class BrowserAuthError extends AuthError {
-    constructor(errorCode: string);
+    constructor(errorCode: string, subError?: string);
 }
 
 declare namespace BrowserAuthErrorCodes {

lib/msal-browser/src/crypto/BrowserCrypto.ts

@@ -9,7 +9,6 @@ import {
 } from "../error/BrowserAuthError.js";
 import {
     IPerformanceClient,
-    Logger,
     PerformanceEvents,
 } from "@azure/msal-common/browser";
 import { KEY_FORMAT_JWK } from "../utils/BrowserConstants.js";
@@ -36,6 +35,8 @@ const UUID_CHARS = "0123456789abcdef";
 // Array to store UINT32 random value
 const UINT32_ARR = new Uint32Array(1);
 
+const SUBTLE_SUBERROR = "crypto_subtle_undefined";
+
 const keygenAlgorithmOptions: RsaHashedKeyGenParams = {
     name: PKCS1_V15_KEYGEN_ALG,
     hash: S256_HASH_ALG,
@@ -46,13 +47,21 @@ const keygenAlgorithmOptions: RsaHashedKeyGenParams = {
 /**
  * Check whether browser crypto is available.
  */
-export function validateCryptoAvailable(logger: Logger): void {
-    if ("crypto" in window) {
-        logger.verbose("BrowserCrypto: modern crypto interface available");
-    } else {
-        logger.error("BrowserCrypto: crypto interface is unavailable");
+export function validateCryptoAvailable(): void {
+    if (!window) {
+        throw createBrowserAuthError(
+            BrowserAuthErrorCodes.nonBrowserEnvironment
+        );
+    }
+    if (!window.crypto) {
         throw createBrowserAuthError(BrowserAuthErrorCodes.cryptoNonExistent);
     }
+    if (!window.crypto.subtle) {
+        throw createBrowserAuthError(
+            BrowserAuthErrorCodes.cryptoNonExistent,
+            SUBTLE_SUBERROR
+        );
+    }
 }
 
 /**

lib/msal-browser/src/crypto/CryptoOps.ts

@@ -53,7 +53,7 @@ export class CryptoOps implements ICrypto {
     constructor(logger: Logger, performanceClient?: IPerformanceClient) {
         this.logger = logger;
         // Browser crypto needs to be validated first before any other classes can be set.
-        BrowserCrypto.validateCryptoAvailable(logger);
+        BrowserCrypto.validateCryptoAvailable();
         this.cache = new AsyncMemoryStorage<CachedKeyPair>(this.logger);
         this.performanceClient = performanceClient;
     }

lib/msal-browser/src/error/BrowserAuthError.ts

@@ -351,14 +351,17 @@ export const BrowserAuthErrorMessage = {
  * Browser library error class thrown by the MSAL.js library for SPAs
  */
 export class BrowserAuthError extends AuthError {
-    constructor(errorCode: string) {
-        super(errorCode, BrowserAuthErrorMessages[errorCode]);
+    constructor(errorCode: string, subError?: string) {
+        super(errorCode, BrowserAuthErrorMessages[errorCode], subError);
 
         Object.setPrototypeOf(this, BrowserAuthError.prototype);
         this.name = "BrowserAuthError";
     }
 }
 
-export function createBrowserAuthError(errorCode: string): BrowserAuthError {
-    return new BrowserAuthError(errorCode);
+export function createBrowserAuthError(
+    errorCode: string,
+    subError?: string
+): BrowserAuthError {
+    return new BrowserAuthError(errorCode, subError);
 }

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

@@ -4299,12 +4299,12 @@ const X_MS_LIB_CAPABILITY = "x-ms-lib-capability";
 // src/response/ResponseHandler.ts:430:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
 // src/response/ResponseHandler.ts:431:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
 // src/response/ResponseHandler.ts:432:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
-// src/telemetry/performance/PerformanceClient.ts:886:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
-// src/telemetry/performance/PerformanceClient.ts:886:15 - (tsdoc-param-tag-with-invalid-type) The @param block should not include a JSDoc-style '{type}'
-// src/telemetry/performance/PerformanceClient.ts:898:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
-// src/telemetry/performance/PerformanceClient.ts:898:27 - (tsdoc-param-tag-with-invalid-type) The @param block should not include a JSDoc-style '{type}'
-// src/telemetry/performance/PerformanceClient.ts:899:24 - (tsdoc-escape-right-brace) The "}" character should be escaped using a backslash to avoid confusion with a TSDoc inline tag
-// src/telemetry/performance/PerformanceClient.ts:899:17 - (tsdoc-malformed-inline-tag) Expecting a TSDoc tag starting with "{@"
+// src/telemetry/performance/PerformanceClient.ts:916:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
+// src/telemetry/performance/PerformanceClient.ts:916:15 - (tsdoc-param-tag-with-invalid-type) The @param block should not include a JSDoc-style '{type}'
+// src/telemetry/performance/PerformanceClient.ts:928:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
+// src/telemetry/performance/PerformanceClient.ts:928:27 - (tsdoc-param-tag-with-invalid-type) The @param block should not include a JSDoc-style '{type}'
+// src/telemetry/performance/PerformanceClient.ts:929:24 - (tsdoc-escape-right-brace) The "}" character should be escaped using a backslash to avoid confusion with a TSDoc inline tag
+// src/telemetry/performance/PerformanceClient.ts:929:17 - (tsdoc-malformed-inline-tag) Expecting a TSDoc tag starting with "{@"
 // src/telemetry/performance/PerformanceEvent.ts:565:21 - (tsdoc-escape-right-brace) The "}" character should be escaped using a backslash to avoid confusion with a TSDoc inline tag
 // src/telemetry/performance/PerformanceEvent.ts:565:14 - (tsdoc-malformed-inline-tag) Expecting a TSDoc tag starting with "{@"
 // src/telemetry/performance/PerformanceEvent.ts:565:8 - (tsdoc-undefined-tag) The TSDoc tag "@type" is not defined in this configuration

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

@@ -198,22 +198,39 @@ export function compactStack(stack: string, stackMaxSize: number): string[] {
     }
 
     const stackArr = stack.split("\n") || [];
-    if (stackArr.length < 2) {
-        return [];
-    }
 
     const res = [];
-    // Get top N stack lines
-    for (
-        // Skip first line as it may contain PII data
-        let ix = Math.max(stackArr.length - stackMaxSize - 1, 1);
-        ix < stackArr.length;
-        ix++
+
+    // Check for a handful of known, common runtime errors and log them (with redaction where applicable).
+    const firstLine = stackArr[0];
+    if (
+        firstLine.startsWith("TypeError: Cannot read property") ||
+        firstLine.startsWith("TypeError: Cannot read properties of") ||
+        firstLine.startsWith("TypeError: Cannot set property") ||
+        firstLine.startsWith("TypeError: Cannot set properties of") ||
+        firstLine.endsWith("is not a function")
     ) {
-        const line = stackArr[ix];
+        // These types of errors are not at risk of leaking PII. They will indicate unavailable APIs
+        res.push(compactStackLine(firstLine));
+    } else if (
+        firstLine.startsWith("SyntaxError") ||
+        firstLine.startsWith("TypeError")
+    ) {
+        // Prevent unintentional leaking of arbitrary info by redacting contents between both single and double quotes
+        res.push(
+            compactStackLine(
+                // Example: SyntaxError: Unexpected token 'e', "test" is not valid JSON -> SyntaxError: Unexpected token <redacted>, <redacted> is not valid JSON
+                firstLine.replace(/['].*[']|["].*["]/g, "<redacted>")
+            )
+        );
+    }
+
+    // Get top N stack lines
+    for (let ix = 1; ix < stackArr.length; ix++) {
         if (res.length >= stackMaxSize) {
             break;
         }
+        const line = stackArr[ix];
         res.push(compactStackLine(line));
     }
     return res;
@@ -630,14 +647,27 @@ export abstract class PerformanceClient implements IPerformanceClient {
             event.correlationId
         );
 
+        if (error) {
+            addError(error, this.logger, rootEvent);
+        }
+
         // Add sub-measurement attribute to root event.
         if (!isRoot) {
             rootEvent[event.name + "DurationMs"] = Math.floor(event.durationMs);
             return { ...rootEvent };
         }
 
-        if (error) {
-            addError(error, this.logger, rootEvent);
+        if (
+            isRoot &&
+            !error &&
+            (rootEvent.errorCode || rootEvent.subErrorCode)
+        ) {
+            this.logger.trace(
+                `PerformanceClient: Remove error and sub-error codes for root event ${event.name} as intermediate error was successfully handled`,
+                event.correlationId
+            );
+            rootEvent.errorCode = undefined;
+            rootEvent.subErrorCode = undefined;
         }
 
         let finalEvent: PerformanceEvent = { ...rootEvent, ...event };