Skip to content

Commit

Permalink
Merge pull request #1313 from authts/fix-1312
Browse files Browse the repository at this point in the history
fix: handle promise in events raise
  • Loading branch information
pamapa committed Jan 9, 2024
2 parents 75f37f9 + 5af4847 commit 5a06565
Show file tree
Hide file tree
Showing 13 changed files with 59 additions and 59 deletions.
12 changes: 6 additions & 6 deletions docs/oidc-client-ts.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -1022,25 +1022,25 @@ export class UserManagerEvents extends AccessTokenEvents {
addUserSignedOut(cb: UserSignedOutCallback): () => void;
addUserUnloaded(cb: UserUnloadedCallback): () => void;
// (undocumented)
load(user: User, raiseEvent?: boolean): void;
load(user: User, raiseEvent?: boolean): Promise<void>;
// (undocumented)
protected readonly _logger: Logger;
// @internal (undocumented)
_raiseSilentRenewError(e: Error): void;
_raiseSilentRenewError(e: Error): Promise<void>;
// @internal (undocumented)
_raiseUserSessionChanged(): void;
_raiseUserSessionChanged(): Promise<void>;
// @internal (undocumented)
_raiseUserSignedIn(): void;
_raiseUserSignedIn(): Promise<void>;
// @internal (undocumented)
_raiseUserSignedOut(): void;
_raiseUserSignedOut(): Promise<void>;
removeSilentRenewError(cb: SilentRenewErrorCallback): void;
removeUserLoaded(cb: UserLoadedCallback): void;
removeUserSessionChanged(cb: UserSessionChangedCallback): void;
removeUserSignedIn(cb: UserSignedInCallback): void;
removeUserSignedOut(cb: UserSignedOutCallback): void;
removeUserUnloaded(cb: UserUnloadedCallback): void;
// (undocumented)
unload(): void;
unload(): Promise<void>;
}

// @public
Expand Down
8 changes: 4 additions & 4 deletions src/SessionMonitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ export class SessionMonitor {
this._checkSessionIFrame.start(session.session_state);

logger.debug("same sub still logged in at OP, session state has changed, restarting check session iframe; session_state", session.session_state);
this._userManager.events._raiseUserSessionChanged();
await this._userManager.events._raiseUserSessionChanged();
}
else {
logger.debug("different subject signed into OP", session.sub);
Expand All @@ -160,10 +160,10 @@ export class SessionMonitor {

if (raiseEvent) {
if (this._sub) {
this._userManager.events._raiseUserSignedOut();
await this._userManager.events._raiseUserSignedOut();
}
else {
this._userManager.events._raiseUserSignedIn();
await this._userManager.events._raiseUserSignedIn();
}
} else {
logger.debug("no change in session detected, no event to raise");
Expand All @@ -172,7 +172,7 @@ export class SessionMonitor {
catch (err) {
if (this._sub) {
logger.debug("Error calling queryCurrentSigninSession; raising signed out event", err);
this._userManager.events._raiseUserSignedOut();
await this._userManager.events._raiseUserSignedOut();
}
}
};
Expand Down
2 changes: 1 addition & 1 deletion src/SilentRenewService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export class SilentRenewService {
}

logger.error("Error from signinSilent:", err);
this._userManager.events._raiseSilentRenewError(err as Error);
await this._userManager.events._raiseSilentRenewError(err as Error);
}
};
}
6 changes: 3 additions & 3 deletions src/UserManager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,14 +93,14 @@ describe("UserManager", () => {
});

describe("getUser", () => {
it("should be able to call getUser without recursion", () => {
it("should be able to call getUser without recursion", async () => {
// arrange
subject.events.addUserLoaded(async () => {
await subject.getUser();
});

// act
subject.events.load({} as User);
await subject.events.load({} as User);
});

it("should return user if there is a user stored", async () => {
Expand Down Expand Up @@ -314,7 +314,7 @@ describe("UserManager", () => {
scope: "openid profile email",
};
jest.spyOn(subject["_client"], "processResourceOwnerPasswordCredentials").mockResolvedValue(mockUser as SigninResponse);
jest.spyOn(subject["_events"], "load").mockReturnValue();
jest.spyOn(subject["_events"], "load").mockImplementation(() => Promise.resolve());

// act
const user:User = await subject.signinResourceOwnerCredentials({ username: "u", password: "p" });
Expand Down
10 changes: 5 additions & 5 deletions src/UserManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ export class UserManager {
const user = await this._loadUser();
if (user) {
logger.info("user loaded");
this._events.load(user, false);
await this._events.load(user, false);
return user;
}

Expand All @@ -154,7 +154,7 @@ export class UserManager {
const logger = this._logger.create("removeUser");
await this.storeUser(null);
logger.info("user removed from storage");
this._events.unload();
await this._events.unload();
}

/**
Expand Down Expand Up @@ -337,7 +337,7 @@ export class UserManager {
const user = new User({ ...args.state, ...response });

await this.storeUser(user);
this._events.load(user);
await this._events.load(user);
return user;
}

Expand Down Expand Up @@ -516,7 +516,7 @@ export class UserManager {

await this.storeUser(user);
logger.debug("user stored");
this._events.load(user);
await this._events.load(user);

return user;
}
Expand Down Expand Up @@ -718,7 +718,7 @@ export class UserManager {

await this.storeUser(user);
logger.debug("user stored");
this._events.load(user);
await this._events.load(user);
}

/**
Expand Down
12 changes: 6 additions & 6 deletions src/UserManagerEvents.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,32 +19,32 @@ describe("UserManagerEvents", () => {

describe("silent renew error", () => {

it("should allow callback", () => {
it("should allow callback", async () => {
// arrange
const cb = jest.fn();

// act
subject.addSilentRenewError(cb);
subject._raiseSilentRenewError(new Error("boom"));
await subject._raiseSilentRenewError(new Error("boom"));

// assert
expect(cb).toBeCalled();
});

it("should allow unregistering callback", () => {
it("should allow unregistering callback", async () => {
// arrange
const cb = jest.fn();

// act
subject.addSilentRenewError(cb);
subject.removeSilentRenewError(cb);
subject._raiseSilentRenewError(new Error("boom"));
await subject._raiseSilentRenewError(new Error("boom"));

// assert
expect(cb).toBeCalledTimes(0);
});

it("should pass error to callback", () => {
it("should pass error to callback", async () => {
// arrange
let e: Error | null = null;
const cb = function (arg_e: Error) {
Expand All @@ -54,7 +54,7 @@ describe("UserManagerEvents", () => {

// act
subject.addSilentRenewError(cb);
subject._raiseSilentRenewError(expected);
await subject._raiseSilentRenewError(expected);

// assert
expect(e).toEqual(expected);
Expand Down
24 changes: 12 additions & 12 deletions src/UserManagerEvents.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,15 @@ export class UserManagerEvents extends AccessTokenEvents {
super({ expiringNotificationTimeInSeconds: settings.accessTokenExpiringNotificationTimeInSeconds });
}

public load(user: User, raiseEvent=true): void {
public async load(user: User, raiseEvent=true): Promise<void> {
super.load(user);
if (raiseEvent) {
this._userLoaded.raise(user);
await this._userLoaded.raise(user);
}
}
public unload(): void {
public async unload(): Promise<void> {
super.unload();
this._userUnloaded.raise();
await this._userUnloaded.raise();
}

/**
Expand Down Expand Up @@ -100,8 +100,8 @@ export class UserManagerEvents extends AccessTokenEvents {
/**
* @internal
*/
public _raiseSilentRenewError(e: Error): void {
this._silentRenewError.raise(e);
public async _raiseSilentRenewError(e: Error): Promise<void> {
await this._silentRenewError.raise(e);
}

/**
Expand All @@ -120,8 +120,8 @@ export class UserManagerEvents extends AccessTokenEvents {
/**
* @internal
*/
public _raiseUserSignedIn(): void {
this._userSignedIn.raise();
public async _raiseUserSignedIn(): Promise<void> {
await this._userSignedIn.raise();
}

/**
Expand All @@ -140,8 +140,8 @@ export class UserManagerEvents extends AccessTokenEvents {
/**
* @internal
*/
public _raiseUserSignedOut(): void {
this._userSignedOut.raise();
public async _raiseUserSignedOut(): Promise<void> {
await this._userSignedOut.raise();
}

/**
Expand All @@ -160,7 +160,7 @@ export class UserManagerEvents extends AccessTokenEvents {
/**
* @internal
*/
public _raiseUserSessionChanged(): void {
this._userSessionChanged.raise();
public async _raiseUserSessionChanged(): Promise<void> {
await this._userSessionChanged.raise();
}
}
4 changes: 2 additions & 2 deletions src/navigators/IFrameWindow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export class IFrameWindow extends AbstractChildWindow {

public async navigate(params: NavigateParams): Promise<NavigateResponse> {
this._logger.debug("navigate: Using timeout of:", this._timeoutInSeconds);
const timer = setTimeout(() => this._abort.raise(new ErrorTimeout("IFrame timed out without a response")), this._timeoutInSeconds * 1000);
const timer = setTimeout(() => void this._abort.raise(new ErrorTimeout("IFrame timed out without a response")), this._timeoutInSeconds * 1000);
this._disposeHandlers.add(() => clearTimeout(timer));

return await super.navigate(params);
Expand All @@ -61,7 +61,7 @@ export class IFrameWindow extends AbstractChildWindow {
this._frame.addEventListener("load", (ev) => {
const frame = ev.target as HTMLIFrameElement;
frame.parentNode?.removeChild(frame);
this._abort.raise(new Error("IFrame removed from DOM"));
void this._abort.raise(new Error("IFrame removed from DOM"));
}, true);
this._frame.contentWindow?.location.replace("about:blank");
}
Expand Down
6 changes: 3 additions & 3 deletions src/navigators/PopupWindow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export class PopupWindow extends AbstractChildWindow {
if (popupWindowFeatures.closePopupWindowAfterInSeconds && popupWindowFeatures.closePopupWindowAfterInSeconds > 0) {
setTimeout(() => {
if (!this._window || typeof this._window.closed !== "boolean" || this._window.closed) {
this._abort.raise(new Error("Popup blocked by user"));
void this._abort.raise(new Error("Popup blocked by user"));
return;
}

Expand All @@ -49,7 +49,7 @@ export class PopupWindow extends AbstractChildWindow {

const popupClosedInterval = setInterval(() => {
if (!this._window || this._window.closed) {
this._abort.raise(new Error("Popup closed by user"));
void this._abort.raise(new Error("Popup closed by user"));
}
}, checkForPopupClosedInterval);
this._disposeHandlers.add(() => clearInterval(popupClosedInterval));
Expand All @@ -61,7 +61,7 @@ export class PopupWindow extends AbstractChildWindow {
if (this._window) {
if (!this._window.closed) {
this._window.close();
this._abort.raise(new Error("Popup closed"));
void this._abort.raise(new Error("Popup closed"));
}
}
this._window = null;
Expand Down
20 changes: 10 additions & 10 deletions src/utils/Event.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,19 @@ describe("Event", () => {

describe("addHandler", () => {

it("should allow callback to be invoked", () => {
it("should allow callback to be invoked", async () => {
// arrange
const cb = jest.fn();

// act
subject.addHandler(cb);
subject.raise();
await subject.raise();

// assert
expect(cb).toBeCalled();
});

it("should allow multiple callbacks", () => {
it("should allow multiple callbacks", async () => {
// arrange
const cb = jest.fn();

Expand All @@ -34,7 +34,7 @@ describe("Event", () => {
subject.addHandler(cb);
subject.addHandler(cb);
subject.addHandler(cb);
subject.raise();
await subject.raise();

// assert
expect(cb).toBeCalledTimes(4);
Expand All @@ -43,20 +43,20 @@ describe("Event", () => {

describe("removeHandler", () => {

it("should remove callback from being invoked", () => {
it("should remove callback from being invoked", async () => {
// arrange
const cb = jest.fn();

// act
subject.addHandler(cb);
subject.removeHandler(cb);
subject.raise();
await subject.raise();

// assert
expect(cb).toBeCalledTimes(0);
});

it("should remove individual callback", () => {
it("should remove individual callback", async () => {
// arrange
const cb1 = jest.fn();
const cb2 = jest.fn();
Expand All @@ -68,7 +68,7 @@ describe("Event", () => {
subject.removeHandler(cb1);
subject.removeHandler(cb1);

subject.raise();
await subject.raise();

// assert
expect(cb1).toBeCalledTimes(0);
Expand All @@ -78,7 +78,7 @@ describe("Event", () => {

describe("raise", () => {

it("should pass params", () => {
it("should pass params", async () => {
// arrange
const typedSubject = subject as Event<[number, number, number]>;
let a = 10;
Expand All @@ -92,7 +92,7 @@ describe("Event", () => {
typedSubject.addHandler(cb);

// act
typedSubject.raise(1, 2, 3);
await typedSubject.raise(1, 2, 3);

// assert
expect(a).toEqual(1);
Expand Down
4 changes: 2 additions & 2 deletions src/utils/Event.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ export class Event<EventType extends unknown[]> {
}
}

public raise(...ev: EventType): void {
public async raise(...ev: EventType): Promise<void> {
this._logger.debug("raise:", ...ev);
for (const cb of this._callbacks) {
void cb(...ev);
await cb(...ev);
}
}
}
Loading

0 comments on commit 5a06565

Please sign in to comment.