Skip to content

Commit a07373d

Browse files
Copilottnorling
Copilot
and
tnorling
authored
Port monitor_window_timeout telemetry enhancements to v5 (dev branch) (#8385)
This pull request enhances telemetry and monitoring in the MSAL Browser library by adding more detailed performance and error tracking, particularly for bridge timeouts and network conditions. It also introduces warnings and telemetry when the redirect URI’s origin does not match the current page, helping to diagnose authentication issues. The changes include propagating the `performanceClient` for more granular telemetry, collecting network information, and expanding test coverage to ensure these features work as intended. **Telemetry and Performance Improvements:** - Added the `performanceClient` parameter to the `waitForBridgeResponse` function and all its call sites in `PopupClient` and `SilentIframeClient`, enabling more detailed telemetry for bridge timeout scenarios. [[1]](diffhunk://#diff-78bb6bdde3811910ee513b0902523f82816adfed7fb4e71c34554ba04711956eL237-R255) [[2]](diffhunk://#diff-3f43afd5556603a80064728bd701519ec2e22979f09ae6095b7fdea0507ad593L375-R376) [[3]](diffhunk://#diff-3f43afd5556603a80064728bd701519ec2e22979f09ae6095b7fdea0507ad593L504-R506) [[4]](diffhunk://#diff-3f43afd5556603a80064728bd701519ec2e22979f09ae6095b7fdea0507ad593L632-R635) [[5]](diffhunk://#diff-3f43afd5556603a80064728bd701519ec2e22979f09ae6095b7fdea0507ad593L786-R790) [[6]](diffhunk://#diff-379febb046eaaa641bafb36c0a72f4c585eda5881b889dd8942919112539e5faL304-R305) [[7]](diffhunk://#diff-379febb046eaaa641bafb36c0a72f4c585eda5881b889dd8942919112539e5faL469-R471) [[8]](diffhunk://#diff-d8aa8313e46503e745028509a1ac18093831ea5baeafe63c63ea825727aa3580L1504-R1504) - Enhanced telemetry in `waitForBridgeResponse` by logging the timeout value and message version, providing more context for monitor window timeout errors. [[1]](diffhunk://#diff-78bb6bdde3811910ee513b0902523f82816adfed7fb4e71c34554ba04711956eL237-R255) [[2]](diffhunk://#diff-78bb6bdde3811910ee513b0902523f82816adfed7fb4e71c34554ba04711956eR286-R297) - Collected network information (effective connection type and round-trip time) in performance events using a new `getNetworkInfo` utility, and included this data in telemetry for better diagnostics. [[1]](diffhunk://#diff-7bf3f5ead1c2093dd056541885a7effa8b79d493ebfac484c2bce9e132cb42e6R21) [[2]](diffhunk://#diff-686e6e522b4c2193064ffc1229f6f8c210bddf5fdc88690acc6ce1f893e4e44cR8-R34) [[3]](diffhunk://#diff-7bf3f5ead1c2093dd056541885a7effa8b79d493ebfac484c2bce9e132cb42e6R164-R172) **Error Handling and Diagnostics:** - Added a warning and a telemetry field when the redirect URI’s origin does not match the current page, helping to surface potential authentication misconfigurations. **Testing Enhancements:** - Expanded unit tests to verify logging and telemetry for cross-origin redirect URIs and the inclusion of network information in performance events, as well as to ensure the new `performanceClient` parameter is properly handled in all relevant test cases. [[1]](diffhunk://#diff-38bf5f23ba532c32331992082c13e9fb5e3e6b61a8ddc5a06f6cb41c5191f29bR346-R396) [[2]](diffhunk://#diff-910035273036aec3a2ea5df6d270f63c5008957027756ead62325c6214ee168aR77-R131) [[3]](diffhunk://#diff-0e4f86d8a16dd8be09b5f3b33d82dafe4c4325e3fd92b994120b0290beb0c2d6L1911-R1912) [[4]](diffhunk://#diff-0e4f86d8a16dd8be09b5f3b33d82dafe4c4325e3fd92b994120b0290beb0c2d6L1947-R1949) [[5]](diffhunk://#diff-0e4f86d8a16dd8be09b5f3b33d82dafe4c4325e3fd92b994120b0290beb0c2d6L1987-R1990) [[6]](diffhunk://#diff-0e4f86d8a16dd8be09b5f3b33d82dafe4c4325e3fd92b994120b0290beb0c2d6L2044-R2056) [[7]](diffhunk://#diff-d597c4d5e0f6d07f5ddd39328c482759a4377c4024b73a1f03c5af03b62bbd07L124-R125) [[8]](diffhunk://#diff-d597c4d5e0f6d07f5ddd39328c482759a4377c4024b73a1f03c5af03b62bbd07L159-R161) [[9]](diffhunk://#diff-d597c4d5e0f6d07f5ddd39328c482759a4377c4024b73a1f03c5af03b62bbd07L198-R201) [[10]](diffhunk://#diff-d597c4d5e0f6d07f5ddd39328c482759a4377c4024b73a1f03c5af03b62bbd07L254-R266) **Documentation and Release:** - Updated API documentation to reflect the new `performanceClient` parameter and created change files for `@azure/msal-browser` and `@azure/msal-common` noting the additional telemetry for monitor window timeout errors. [[1]](diffhunk://#diff-d8aa8313e46503e745028509a1ac18093831ea5baeafe63c63ea825727aa3580L1504-R1504) [[2]](diffhunk://#diff-a0ad68ef5aa0fbf7b3181f222cdc9314009d150df48e59d6de586a2c0bec5840R1-R7) [[3]](diffhunk://#diff-528f1b737ee6dde3d5da4d005008c0edc57475bc94174ef051f520ed1f504d0aR1-R7) --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tnorling <5307810+tnorling@users.noreply.github.com> Co-authored-by: Thomas Norling <thomas.norling@microsoft.com>
1 parent 543e435 commit a07373d

17 files changed

Lines changed: 455 additions & 534 deletions

‎change/@azure-msal-browser-43f9e5a9-94f7-4463-a4e9-fca092b41cad.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "Additional telemetry for monitor_window_timeout errors [#8385](https://github.com/AzureAD/microsoft-authentication-library-for-js/pull/8385)",
4+
"packageName": "@azure/msal-browser",
5+
"email": "thomas.norling@microsoft.com",
6+
"dependentChangeType": "patch"
7+
}

‎change/@azure-msal-common-7ef9342e-351a-4732-857c-4df5d7b4216b.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "Additional telemetry for monitor_window_timeout errors [#8385](https://github.com/AzureAD/microsoft-authentication-library-for-js/pull/8385)",
4+
"packageName": "@azure/msal-common",
5+
"email": "thomas.norling@microsoft.com",
6+
"dependentChangeType": "patch"
7+
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1501,7 +1501,7 @@ export const version = "5.4.0";
15011501
// Warning: (ae-missing-release-tag) "waitForBridgeResponse" is part of the package's API, but it is missing a release tag (@alpha, @beta, @public, or @internal)
15021502
//
15031503
// @public (undocumented)
1504-
function waitForBridgeResponse(timeoutMs: number, logger: Logger, browserCrypto: ICrypto, request: CommonAuthorizationUrlRequest | CommonEndSessionRequest): Promise<string>;
1504+
function waitForBridgeResponse(timeoutMs: number, logger: Logger, browserCrypto: ICrypto, request: CommonAuthorizationUrlRequest | CommonEndSessionRequest, performanceClient: IPerformanceClient): Promise<string>;
15051505

15061506
// Warning: (ae-missing-release-tag) "WrapperSKU" is part of the package's API, but it is missing a release tag (@alpha, @beta, @public, or @internal)
15071507
// Warning: (ae-missing-release-tag) "WrapperSKU" is part of the package's API, but it is missing a release tag (@alpha, @beta, @public, or @internal)

‎lib/msal-browser/src/interaction_client/PopupClient.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -372,7 +372,8 @@ export class PopupClient extends StandardInteractionClient {
372372
this.config.system.popupBridgeTimeout,
373373
this.logger,
374374
this.browserCrypto,
375-
request
375+
request,
376+
this.performanceClient
376377
);
377378

378379
const serverParams = invoke(
@@ -501,7 +502,8 @@ export class PopupClient extends StandardInteractionClient {
501502
this.config.system.popupBridgeTimeout,
502503
this.logger,
503504
this.browserCrypto,
504-
popupRequest
505+
popupRequest,
506+
this.performanceClient
505507
);
506508

507509
const serverParams = invoke(
@@ -629,7 +631,8 @@ export class PopupClient extends StandardInteractionClient {
629631
this.config.system.popupBridgeTimeout,
630632
this.logger,
631633
this.browserCrypto,
632-
request
634+
request,
635+
this.performanceClient
633636
);
634637

635638
const serverParams = invoke(
@@ -783,7 +786,8 @@ export class PopupClient extends StandardInteractionClient {
783786
this.config.system.popupBridgeTimeout,
784787
this.logger,
785788
this.browserCrypto,
786-
validRequest
789+
validRequest,
790+
this.performanceClient
787791
).catch(() => {
788792
// Swallow any errors related to monitoring the window. Server logout is best effort
789793
});

‎lib/msal-browser/src/interaction_client/SilentIframeClient.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,8 @@ export class SilentIframeClient extends StandardInteractionClient {
301301
this.config.system.iframeBridgeTimeout,
302302
this.logger,
303303
this.browserCrypto,
304-
request
304+
request,
305+
this.performanceClient
305306
);
306307

307308
const serverParams = invoke(
@@ -466,7 +467,8 @@ export class SilentIframeClient extends StandardInteractionClient {
466467
this.config.system.iframeBridgeTimeout,
467468
this.logger,
468469
this.browserCrypto,
469-
request
470+
request,
471+
this.performanceClient
470472
);
471473

472474
const serverParams = invoke(

‎lib/msal-browser/src/interaction_client/StandardInteractionClient.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,16 @@ export async function initializeAuthorizationRequest(
327327
logger,
328328
correlationId
329329
);
330+
if (new URL(redirectUri).origin !== new URL(window.location.href).origin) {
331+
logger.warning(
332+
"The origin of the redirect URI does not match the origin of the current page. This is likely to cause issues with authentication.",
333+
correlationId
334+
);
335+
performanceClient.addFields(
336+
{ isRedirectUriCrossOrigin: true },
337+
correlationId
338+
);
339+
}
330340
const browserState: BrowserStateObject = {
331341
interactionType: interactionType,
332342
};

‎lib/msal-browser/src/telemetry/BrowserPerformanceClient.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import { name, version } from "../packageMetadata.js";
1818
import { BrowserCacheLocation } from "../utils/BrowserConstants.js";
1919
import * as BrowserCrypto from "../crypto/BrowserCrypto.js";
2020
import { BROWSER_PERF_ENABLED_KEY } from "../cache/CacheKeys.js";
21+
import { getNetworkInfo } from "../utils/MsalFrameStatsUtils.js";
2122

2223
/**
2324
* Returns browser performance measurement module if session flag is enabled. Returns undefined otherwise.
@@ -160,12 +161,15 @@ export class BrowserPerformanceClient
160161
error?: unknown,
161162
account?: AccountInfo
162163
): PerformanceEvent | null => {
164+
const networkInfo = getNetworkInfo();
163165
const res = inProgressEvent.end(
164166
{
165167
...event,
166168
startPageVisibility,
167169
endPageVisibility: this.getPageVisibility(),
168170
durationMs: getPerfDurationMs(startTime),
171+
networkEffectiveType: networkInfo.effectiveType,
172+
networkRtt: networkInfo.rtt,
169173
},
170174
error,
171175
account

‎lib/msal-browser/src/utils/BrowserUtils.ts‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
UrlUtils,
1111
RequestParameterBuilder,
1212
ICrypto,
13+
IPerformanceClient,
1314
Logger,
1415
CommonAuthorizationUrlRequest,
1516
CommonEndSessionRequest,
@@ -234,14 +235,24 @@ export async function waitForBridgeResponse(
234235
timeoutMs: number,
235236
logger: Logger,
236237
browserCrypto: ICrypto,
237-
request: CommonAuthorizationUrlRequest | CommonEndSessionRequest
238+
request: CommonAuthorizationUrlRequest | CommonEndSessionRequest,
239+
performanceClient: IPerformanceClient
238240
): Promise<string> {
239241
return new Promise<string>((resolve, reject) => {
240242
logger.verbose(
241243
"BrowserUtils.waitForBridgeResponse - started",
242244
request.correlationId
243245
);
244246

247+
const correlationId = request.correlationId;
248+
249+
performanceClient.addFields(
250+
{
251+
redirectBridgeTimeoutMs: timeoutMs,
252+
},
253+
correlationId
254+
);
255+
245256
const { libraryState } = ProtocolUtils.parseRequestState(
246257
browserCrypto.base64Decode,
247258
request.state || ""
@@ -272,6 +283,18 @@ export async function waitForBridgeResponse(
272283
channel.onmessage = (event) => {
273284
responseString = event.data.payload;
274285

286+
const messageVersion =
287+
event?.data && typeof event.data.v === "number"
288+
? event.data.v
289+
: undefined;
290+
291+
performanceClient.addFields(
292+
{
293+
redirectBridgeMessageVersion: messageVersion,
294+
},
295+
correlationId
296+
);
297+
275298
// Clear the active monitor
276299
activeBridgeMonitor = null;
277300

‎lib/msal-browser/src/utils/MsalFrameStatsUtils.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,33 @@
55

66
import { InProgressPerformanceEvent, Logger } from "@azure/msal-common/browser";
77

8+
interface NetworkConnection {
9+
effectiveType?: string;
10+
rtt?: number;
11+
}
12+
13+
/**
14+
* Get network information for telemetry purposes. This is only supported in Chromium-based browsers.
15+
* @returns Network connection information, or an empty object if not available.
16+
*/
17+
export function getNetworkInfo(): NetworkConnection {
18+
if (typeof window === "undefined" || !window.navigator) {
19+
return {};
20+
}
21+
const connection: NetworkConnection | undefined =
22+
"connection" in window.navigator
23+
? (
24+
window.navigator as Navigator & {
25+
connection: NetworkConnection;
26+
}
27+
).connection
28+
: undefined;
29+
return {
30+
effectiveType: connection?.effectiveType,
31+
rtt: connection?.rtt,
32+
};
33+
}
34+
835
export function collectInstanceStats(
936
currentClientId: string,
1037
performanceEvent: InProgressPerformanceEvent,

‎lib/msal-browser/test/interaction_client/PopupClient.spec.ts‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1908,7 +1908,8 @@ describe("PopupClient", () => {
19081908
5000,
19091909
clientImpl.logger,
19101910
clientImpl.browserCrypto,
1911-
request
1911+
request,
1912+
clientImpl.performanceClient
19121913
);
19131914

19141915
expect(response).toEqual("code=testCode&state=testState");
@@ -1944,7 +1945,8 @@ describe("PopupClient", () => {
19441945
5000,
19451946
clientImpl.logger,
19461947
clientImpl.browserCrypto,
1947-
request
1948+
request,
1949+
clientImpl.performanceClient
19481950
);
19491951

19501952
expect(response).toEqual("code=authCode&state=testState456");
@@ -1984,7 +1986,8 @@ describe("PopupClient", () => {
19841986
100,
19851987
clientImpl.logger,
19861988
clientImpl.browserCrypto,
1987-
request
1989+
request,
1990+
clientImpl.performanceClient
19881991
)
19891992
).rejects.toMatchObject({
19901993
errorCode: BrowserAuthErrorCodes.timedOut,
@@ -2041,14 +2044,16 @@ describe("PopupClient", () => {
20412044
5000,
20422045
clientImpl.logger,
20432046
clientImpl.browserCrypto,
2044-
request1
2047+
request1,
2048+
clientImpl.performanceClient
20452049
);
20462050

20472051
const promise2 = BrowserUtils.waitForBridgeResponse(
20482052
5000,
20492053
clientImpl.logger,
20502054
clientImpl.browserCrypto,
2051-
request2
2055+
request2,
2056+
clientImpl.performanceClient
20522057
);
20532058

20542059
const [response1, response2] = await Promise.all([

0 commit comments

Comments
 (0)