Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Instrument non-auth error name and stack #6937

Merged
merged 10 commits into from
Mar 10, 2024
Merged

Instrument non-auth error name and stack #6937

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
39e95e1
- Add telemetry functions to capture non-auth error name and stack.
konstantin-msft Mar 5, 2024
eb163df
- Add telemetry functions to capture non-auth error name and stack.
konstantin-msft Mar 5, 2024
b5e09f8
Change files
konstantin-msft Mar 5, 2024
e0e5a27
- Add telemetry functions to capture non-auth error name and stack.
konstantin-msft Mar 6, 2024
a165de9
Change files
konstantin-msft Mar 6, 2024
7b7856c
- Add telemetry functions to capture non-auth error name and stack.
konstantin-msft Mar 7, 2024
ba3dc3c
- Instrument non-auth error name and stack.
konstantin-msft Mar 7, 2024
05d6c62
- Fix lint errors.
konstantin-msft Mar 7, 2024
7c22424
- Revert changes to NativeInteractionClient.ts.
konstantin-msft Mar 7, 2024
68f7962
- Revert changes to test clients.
konstantin-msft Mar 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions change/@azure-msal-browser-476741b2-6b5f-4d05-8d76-355f02784e68.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "Instrument non-auth error name and stack #6937",
"packageName": "@azure/msal-browser",
"email": "kshabelko@microsoft.com",
"dependentChangeType": "patch"
}
7 changes: 7 additions & 0 deletions change/@azure-msal-common-dbb12e75-776c-46b9-a02d-b59c34223c66.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "Instrument non-auth error name and stack #6937",
"packageName": "@azure/msal-common",
"email": "kshabelko@microsoft.com",
"dependentChangeType": "patch"
}
22 changes: 12 additions & 10 deletions lib/msal-browser/src/controllers/NestedAppAuthController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,11 +191,12 @@ export class NestedAppAuthController implements IController {
e as EventError
);

atPopupMeasurement.end({
errorCode: error.errorCode,
subErrorCode: error.subError,
success: false,
});
atPopupMeasurement.end(
{
success: false,
},
e
);

throw error;
}
Expand Down Expand Up @@ -260,11 +261,12 @@ export class NestedAppAuthController implements IController {
null,
e as EventError
);
ssoSilentMeasurement?.end({
errorCode: error.errorCode,
subErrorCode: error.subError,
success: false,
});
ssoSilentMeasurement?.end(
{
success: false,
},
e
);
throw error;
}
}
Expand Down
107 changes: 55 additions & 52 deletions lib/msal-browser/src/controllers/StandardController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,17 +479,13 @@ export class StandardController implements IController {
EventType.HANDLE_REDIRECT_END,
InteractionType.Redirect
);
if (eventError instanceof AuthError) {
rootMeasurement.end({
success: false,
errorCode: eventError.errorCode,
subErrorCode: eventError.subError,
});
} else {
rootMeasurement.end({

rootMeasurement.end(
{
success: false,
});
}
},
eventError
);

throw e;
});
Expand Down Expand Up @@ -723,7 +719,7 @@ export class StandardController implements IController {
});
return result;
})
.catch((e: AuthError) => {
.catch((e: Error) => {
if (loggedInAccounts.length > 0) {
this.eventHandler.emitEvent(
EventType.ACQUIRE_TOKEN_FAILURE,
Expand All @@ -740,11 +736,13 @@ export class StandardController implements IController {
);
}

atPopupMeasurement.end({
errorCode: e.errorCode,
subErrorCode: e.subError,
success: false,
});
atPopupMeasurement.end(
{
success: false,
},
e
);

// Since this function is syncronous we need to reject
return Promise.reject(e);
});
Expand Down Expand Up @@ -854,18 +852,19 @@ export class StandardController implements IController {
});
return response;
})
.catch((e: AuthError) => {
.catch((e: Error) => {
this.eventHandler.emitEvent(
EventType.SSO_SILENT_FAILURE,
InteractionType.Silent,
null,
e
);
this.ssoSilentMeasurement?.end({
errorCode: e.errorCode,
subErrorCode: e.subError,
success: false,
});
this.ssoSilentMeasurement?.end(
{
success: false,
},
e
);
throw e;
})
.finally(() => {
Expand Down Expand Up @@ -938,19 +937,20 @@ export class StandardController implements IController {
});
return result;
})
.catch((error: AuthError) => {
.catch((error: Error) => {
this.hybridAuthCodeResponses.delete(hybridAuthCode);
this.eventHandler.emitEvent(
EventType.ACQUIRE_TOKEN_BY_CODE_FAILURE,
InteractionType.Silent,
null,
error
);
atbcMeasurement.end({
errorCode: error.errorCode,
subErrorCode: error.subError,
success: false,
});
atbcMeasurement.end(
{
success: false,
},
error
);
throw error;
});
this.hybridAuthCodeResponses.set(hybridAuthCode, response);
Expand Down Expand Up @@ -998,12 +998,12 @@ export class StandardController implements IController {
null,
e as EventError
);
atbcMeasurement.end({
errorCode: (e instanceof AuthError && e.errorCode) || undefined,
subErrorCode:
(e instanceof AuthError && e.subError) || undefined,
success: false,
});
atbcMeasurement.end(
{
success: false,
},
e
);
throw e;
}
}
Expand Down Expand Up @@ -1046,12 +1046,13 @@ export class StandardController implements IController {
});
return response;
})
.catch((tokenRenewalError: AuthError) => {
this.acquireTokenByCodeAsyncMeasurement?.end({
errorCode: tokenRenewalError.errorCode,
subErrorCode: tokenRenewalError.subError,
success: false,
});
.catch((tokenRenewalError: Error) => {
this.acquireTokenByCodeAsyncMeasurement?.end(
{
success: false,
},
tokenRenewalError
);
throw tokenRenewalError;
})
.finally(() => {
Expand Down Expand Up @@ -1902,13 +1903,14 @@ export class StandardController implements IController {
});
return result;
})
.catch((error: AuthError) => {
.catch((error: Error) => {
this.activeSilentTokenRequests.delete(silentRequestKey);
atsMeasurement.end({
errorCode: error.errorCode,
subErrorCode: error.subError,
success: false,
});
atsMeasurement.end(
{
success: false,
},
error
);
throw error;
});
this.activeSilentTokenRequests.set(silentRequestKey, response);
Expand Down Expand Up @@ -2091,18 +2093,19 @@ export class StandardController implements IController {
});
return response;
})
.catch((tokenRenewalError: AuthError) => {
.catch((tokenRenewalError: Error) => {
this.eventHandler.emitEvent(
EventType.ACQUIRE_TOKEN_FAILURE,
InteractionType.Silent,
null,
tokenRenewalError
);
this.atsAsyncMeasurement?.end({
errorCode: tokenRenewalError.errorCode,
subErrorCode: tokenRenewalError.subError,
success: false,
});
this.atsAsyncMeasurement?.end(
{
success: false,
},
tokenRenewalError
);
throw tokenRenewalError;
})
.finally(() => {
Expand Down
18 changes: 11 additions & 7 deletions lib/msal-browser/src/telemetry/BrowserPerformanceClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,14 +159,18 @@ export class BrowserPerformanceClient
return {
...inProgressEvent,
end: (
event?: Partial<PerformanceEvent>
event?: Partial<PerformanceEvent>,
error?: unknown
): PerformanceEvent | null => {
const res = inProgressEvent.end({
...event,
startPageVisibility,
endPageVisibility: this.getPageVisibility(),
durationMs: getPerfDurationMs(startTime),
});
const res = inProgressEvent.end(
{
...event,
startPageVisibility,
endPageVisibility: this.getPageVisibility(),
durationMs: getPerfDurationMs(startTime),
},
error
);
void browserMeasurement?.then((measurement) =>
measurement.endMeasurement()
);
Expand Down
56 changes: 44 additions & 12 deletions lib/msal-browser/test/app/PublicClientApplication.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import {
InteractionRequiredAuthErrorCodes,
Logger,
LogLevel,
PerformanceEvent,
PerformanceEvents,
PersistentCacheKeys,
ProtocolMode,
Expand Down Expand Up @@ -89,6 +90,7 @@ import { RedirectClient } from "../../src/interaction_client/RedirectClient";
import { PopupClient } from "../../src/interaction_client/PopupClient";
import { SilentCacheClient } from "../../src/interaction_client/SilentCacheClient";
import { SilentRefreshClient } from "../../src/interaction_client/SilentRefreshClient";
import { BaseInteractionClient } from "../../src/interaction_client/BaseInteractionClient";
import { AuthorizationCodeRequest, EndSessionRequest } from "../../src";
import { RedirectHandler } from "../../src/interaction_handler/RedirectHandler";
import { SilentAuthCodeClient } from "../../src/interaction_client/SilentAuthCodeClient";
Expand Down Expand Up @@ -2810,10 +2812,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
};
const silentClientSpy = sinon
.stub(SilentIframeClient.prototype, "acquireToken")
.rejects({
errorCode: "abc",
subError: "defg",
});
.rejects(new AuthError("abc", "error message", "defg"));
const callbackId = pca.addPerformanceCallback((events) => {
expect(events[0].correlationId).toBe(RANDOM_TEST_GUID);
expect(events[0].success).toBe(false);
Expand Down Expand Up @@ -3269,10 +3268,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
};
const silentClientSpy = sinon
.stub(SilentAuthCodeClient.prototype, "acquireToken")
.rejects({
errorCode: "abc",
subError: "defg",
});
.rejects(new AuthError("abc", "error message", "defg"));
const callbackId = pca.addPerformanceCallback((events) => {
expect(events[0].correlationId).toBe(RANDOM_TEST_GUID);
expect(events[0].success).toBe(false);
Expand Down Expand Up @@ -3554,6 +3550,45 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
expect(silentIframeSpy.called).toBe(false);
});

it("Calls SilentCacheClient.acquireToken and captures the stack trace for non-auth error", (done) => {
const testAccount: AccountInfo = {
homeAccountId: TEST_DATA_CLIENT_INFO.TEST_HOME_ACCOUNT_ID,
localAccountId: TEST_DATA_CLIENT_INFO.TEST_UID,
environment: "login.windows.net",
tenantId: "3338040d-6c67-4c5b-b112-36a304b66dad",
username: "AbeLi@microsoft.com",
};

sinon
// @ts-ignore
.stub(BaseInteractionClient.prototype, "initializeBaseRequest")
.callsFake(async () => {
throw new Error("Test error message");
});

const callbackId = pca.addPerformanceCallback(
(events: PerformanceEvent[]) => {
expect(events.length).toEqual(1);
const event = events[0];
expect(event.name).toBe(
PerformanceEvents.AcquireTokenSilent
);
expect(event.correlationId).toBeDefined();
expect(event.success).toBeFalsy();
expect(event.errorName).toEqual("Error");
expect(event.errorStack?.length).toEqual(5);
pca.removePerformanceCallback(callbackId);
done();
}
);

pca.acquireTokenSilent({
scopes: ["openid"],
account: testAccount,
state: "test-state",
}).catch(() => {});
});

it("Calls SilentRefreshClient.acquireToken and returns its response if cache lookup throws", async () => {
const testAccount: AccountInfo = {
homeAccountId: TEST_DATA_CLIENT_INFO.TEST_HOME_ACCOUNT_ID,
Expand Down Expand Up @@ -4829,10 +4864,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
StandardController.prototype,
<any>"acquireTokenSilentAsync"
)
.rejects({
errorCode: "abc",
subError: "defg",
});
.rejects(new AuthError("abc", "error message", "defg"));

const callbackId = pca.addPerformanceCallback((events) => {
expect(events[0].correlationId).toBe(RANDOM_TEST_GUID);
Expand Down
5 changes: 4 additions & 1 deletion lib/msal-common/src/telemetry/performance/IPerformanceClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ import { IPerformanceMeasurement } from "./IPerformanceMeasurement";
export type PerformanceCallbackFunction = (events: PerformanceEvent[]) => void;

export type InProgressPerformanceEvent = {
end: (event?: Partial<PerformanceEvent>) => PerformanceEvent | null;
end: (
event?: Partial<PerformanceEvent>,
error?: unknown
) => PerformanceEvent | null;
discard: () => void;
add: (fields: { [key: string]: {} | undefined }) => void;
increment: (fields: { [key: string]: number | undefined }) => void;
Expand Down
Loading
Loading