Ensure await on storeEntity and handle async operations correctly

packages/backend/src/actors/FingerprintStore.ts

@@ -50,14 +50,11 @@ export class FingerprintStore extends SubscribableActor<Fingerprint, Fingerprint
 		}
 		const result = await FingerprintStoreMessages.match<Promise<FingerprintStoreResult>>(message, {
 			StoreFingerprint: async fingerprint => {
-				this.state = await create(this.state, async draft => {
-					const currentFingerprint = (await this.getEntity(fingerprint.uid)).orElse({} as Fingerprint);
-					fingerprint.updated = toTimestamp();
-					const newFingerprint = { ...currentFingerprint, ...fingerprint };
-					draft.cache.set(fingerprint.uid, newFingerprint);
-					logger.debug(`Storing new fingerprint ${JSON.stringify(newFingerprint)}`);
-					this.storeEntity(newFingerprint);
-				});
+				const currentFingerprint = (await this.getEntity(fingerprint.uid)).orElse({} as Fingerprint);
+				fingerprint.updated = toTimestamp();
+				const newFingerprint = { ...currentFingerprint, ...fingerprint };
+				logger.debug(`Storing new fingerprint ${JSON.stringify(newFingerprint)}`);
+				await this.storeEntity(newFingerprint);
 				return unit();
 			},
 			Get: async id => {
@@ -66,49 +63,42 @@ export class FingerprintStore extends SubscribableActor<Fingerprint, Fingerprint
 				return Promise.resolve(retVal)
 			},
 			Block: async id => {
-				const mbFingerprint = await this.getEntity(id)
-				const {uid} = await this.ask<UserStoreMessage, User>("actors://recapp-backend/UserStore", UserStoreMessages.GetByFingerprint(id));
-				return mbFingerprint.match<Unit|Error>(
-					fp => {
-						this.storeEntity({...fp, blocked: true});
-						this.updateSubscribers({...fp, blocked: true})
-						if (uid)
-							this.send("actors://recapp-backend/UserStore", UserStoreMessages.Update({ uid, active: false }));
-						return unit();
-					},
-					() => {
-						return new Error(`Unknown fingerprint id ${id}`);
-					}
-				)
+				const fp = (await this.getEntity(id)).orUndefined();
+				if (!fp) {
+					return new Error(`Unknown fingerprint id ${id}`);
+				}
+				const { uid } = await this.ask<UserStoreMessage, User>("actors://recapp-backend/UserStore", UserStoreMessages.GetByFingerprint(id));
+				const updated = { ...fp, blocked: true };
+				await this.storeEntity(updated);
+				this.updateSubscribers(updated);
+				if (uid) {
+					this.send("actors://recapp-backend/UserStore", UserStoreMessages.Update({ uid, active: false }));
+				}
+				return unit();
 			},
 			Unblock: async id => {
-				const mbFingerprint = await this.getEntity(id)
-				const {uid} = await this.ask<UserStoreMessage, User>("actors://recapp-backend/UserStore", UserStoreMessages.GetByFingerprint(id));
-				return mbFingerprint.match<Unit|Error>(
-					fp => {
-						this.storeEntity({...fp, blocked: false});
-						this.updateSubscribers({...fp, blocked: false})
-						if (uid)
-							this.send("actors://recapp-backend/UserStore", UserStoreMessages.Update({ uid, active: true }));
-						return unit();
-					},
-					() => {
-						return new Error(`Unknown fingerprint id ${id}`);
-					}
-				)
+				const fp = (await this.getEntity(id)).orUndefined();
+				if (!fp) {
+					return new Error(`Unknown fingerprint id ${id}`);
+				}
+				const { uid } = await this.ask<UserStoreMessage, User>("actors://recapp-backend/UserStore", UserStoreMessages.GetByFingerprint(id));
+				const updated = { ...fp, blocked: false };
+				await this.storeEntity(updated);
+				this.updateSubscribers(updated);
+				if (uid) {
+					this.send("actors://recapp-backend/UserStore", UserStoreMessages.Update({ uid, active: true }));
+				}
+				return unit();
 			},
 			IncreaseCount: async ({fingerprint, userUid, initialQuiz}) => {
-				const mbFingerprint = await this.getEntity(fingerprint)
-				return mbFingerprint.match<Unit|Error>(
-					fp => {
-						this.storeEntity({...fp, usageCount: fp.usageCount + 1, lastSeen: toTimestamp(), userUid, initialQuiz: initialQuiz ?? fp.initialQuiz});
-						this.updateSubscribers({...fp, usageCount: fp.usageCount + 1, lastSeen: toTimestamp(), userUid, initialQuiz: initialQuiz ?? fp.initialQuiz})
-						return unit();
-					},
-					() => {
-						return new Error(`Unknown fingerprint id ${fingerprint}`);
-					}
-				)
+				const fp = (await this.getEntity(fingerprint)).orUndefined();
+				if (!fp) {
+					return new Error(`Unknown fingerprint id ${fingerprint}`);
+				}
+				const updated = { ...fp, usageCount: fp.usageCount + 1, lastSeen: toTimestamp(), userUid, initialQuiz: initialQuiz ?? fp.initialQuiz };
+				await this.storeEntity(updated);
+				this.updateSubscribers(updated);
+				return unit();
 			},
 			GetMostRecent: async () => {
 				const db = await this.connector.db();

packages/backend/src/actors/SessionStore.ts

@@ -47,16 +47,16 @@ export class SessionStore extends StoringActor<Session, SessionStoreMessage, Ses
 	public async receive(from: ActorRef, message: SessionStoreMessage): Promise<SessionStoreResult> {
 		const result = await SessionStoreMessages.match<Promise<SessionStoreResult>>(message, {
 			StoreSession: async session => {
-				this.state = await create(this.state, async draft => {
-					const currentSession = (await this.getEntity(session.uid)).orElse({} as Session);
+				const currentSession = (await this.getEntity(session.uid)).orElse({} as Session);
+				session.updated = toTimestamp();
+				const newSession = { ...currentSession, ...session };
+				this.state = create(this.state, draft => {
 					if (session.actorSystem) {
 						draft.clientIndex.set(session.actorSystem, session.uid);
 					}
-					session.updated = toTimestamp();
-					const newSession = { ...currentSession, ...session };
 					draft.cache.set(session.uid, newSession);
-					this.storeEntity(newSession);
 				});
+				await this.storeEntity(newSession);
 				return unit();
 			},
 			CheckSession: async userId => {

packages/backend/src/actors/StatisticsActor.ts

@@ -243,7 +243,7 @@ export class StatisticsActor extends SubscribableActor<
 					);
 					// console.log("STATS", stats);
 					this.logger.info(`STATS stored quizId=${String(this.uid)}`);
-					this.storeEntity(stats);
+					await this.storeEntity(stats);
 					for (const [subscriber] of this.state.collectionSubscribers) {
 						this.send(subscriber, new StatisticsUpdateMessage(stats));
 					}

packages/backend/src/actors/UserStore.ts

@@ -353,26 +353,6 @@ export class UserStore extends SubscribableActor<User, UserStoreMessage, ResultT
 			newUser.updated = toTimestamp();
 			draft.cache.set(userToStore.uid, newUser);
 
-			// If the role has changed, also update the session store
-			if (oldUser.role !== newUser.role) {
-				(
-					this.ask(
-						createActorUri("SessionStore"),
-						SessionStoreMessages.GetSessionForUserId(newUser.uid)
-					) as Promise<Session>
-				)
-					.then((session: Session) => {
-						session.role = newUser.role;
-						this.send(createActorUri("SessionStore"), SessionStoreMessages.StoreSession(session));
-					})
-					.catch((e: unknown) => {
-						this.logger.error(
-							`Failed to update session role for user ${String(newUser.uid)}: ` +
-							`${e instanceof Error ? e.stack : String(e)}`
-						);
-					});
-			}
-
 			for (const [subscriber, subscription] of this.state.collectionSubscribers) {
 				this.send(
 					subscriber,
@@ -389,8 +369,30 @@ export class UserStore extends SubscribableActor<User, UserStoreMessage, ResultT
 		});
 		// console.log("Updated user", newUser);
 		this.logger.info(`USERSTORE updated user uid=${String((newUser as any)?.uid ?? "?")}`);
-		return this.storeEntity(newUser)
+		const storeResult = await this.storeEntity(newUser)
 			.then(() => newUser)
 			.catch(error => error as Error);
+		if (!(storeResult instanceof Error) && oldUser.role !== newUser.role) {
+			try {
+				const sessionOrError = await this.ask(
+					createActorUri("SessionStore"),
+					SessionStoreMessages.GetSessionForUserId(newUser.uid)
+				);
+				if (!(sessionOrError instanceof Error)) {
+					const session = sessionOrError as Session;
+					session.role = newUser.role;
+					await this.ask(
+						createActorUri("SessionStore"),
+						SessionStoreMessages.StoreSession(session)
+					);
+				}
+			} catch (e: unknown) {
+				this.logger.error(
+					`Failed to update session role for user ${String(newUser.uid)}: ` +
+					`${e instanceof Error ? e.stack : String(e)}`
+				);
+			}
+		}
+		return storeResult;
 	};
 }

packages/backend/test/fingerprintStore.test.ts

@@ -0,0 +1,98 @@
+import { Actor, ActorRef, ActorSystem } from "ts-actors";
+import { toTimestamp } from "itu-utils";
+import { vi } from "vitest";
+import { FingerprintStore } from "../src/actors/FingerprintStore";
+import { Fingerprint, FingerprintStoreMessages } from "@recapp/models";
+import { mockDbInstance } from "./setup/mongoMock";
+
+// FingerprintStore.Block asks UserStore for the owning user; a minimal stub keeps the
+// test free of the real UserStore while exercising the awaited block write.
+class UserStoreStub extends Actor<any, any> {
+	public constructor(name: string, system: ActorSystem) {
+		super(name, system);
+	}
+	public async receive(_from: ActorRef, _message: any): Promise<any> {
+		return { uid: "user-1" };
+	}
+}
+
+// ts-actors surfaces a thrown handler error to the registered error receiver (it does not
+// reject the ask). Capturing here lets us assert the awaited write's failure is surfaced.
+const captured: unknown[] = [];
+class ErrorReceiver extends Actor<unknown, unknown> {
+	public constructor(name: string, system: ActorSystem) {
+		super(name, system);
+	}
+	public async receive(_from: ActorRef, message: unknown): Promise<unknown> {
+		captured.push(message);
+		return undefined;
+	}
+}
+
+async function waitFor(pred: () => boolean, timeoutMs = 2000): Promise<void> {
+	const start = Date.now();
+	while (!pred() && Date.now() - start < timeoutMs) {
+		await new Promise(r => setTimeout(r, 10));
+	}
+}
+
+function makeFingerprint(overrides: Partial<Fingerprint> = {}): Fingerprint {
+	const now = toTimestamp();
+	return {
+		uid: "fp-1" as any,
+		created: now,
+		updated: now,
+		lastSeen: now,
+		usageCount: 1,
+		blocked: false,
+		userUid: "user-1" as any,
+		...overrides,
+	};
+}
+
+// Characterises the await-and-call fix on the moderation store: block/unblock and store
+// writes are awaited, so a blocked fingerprint is persisted before the ask resolves and
+// cannot silently revert to unblocked after a restart (the note's headline risk).
+describe("FingerprintStore — awaited persistence", () => {
+	let system: ActorSystem;
+
+	beforeEach(async () => {
+		captured.length = 0;
+		system = await ActorSystem.create({ systemName: "recapp-backend" });
+	});
+
+	afterEach(async () => {
+		await system.shutdown();
+		vi.restoreAllMocks();
+	});
+
+	test("StoreFingerprint commits the fingerprint to the DB before the ask resolves", async () => {
+		const ref = await system.createActor(FingerprintStore, { name: "FingerprintStore" });
+		await system.ask(ref, FingerprintStoreMessages.StoreFingerprint(makeFingerprint()));
+		const doc = await mockDbInstance.collection("fingerprints").findOne({ uid: "fp-1" });
+		expect(doc).not.toBeNull();
+		expect((doc as any).blocked).toBe(false);
+	});
+
+	test("Block persists blocked:true (awaited) so a restart can't revert it", async () => {
+		await system.createActor(UserStoreStub, { name: "UserStore" });
+		const ref = await system.createActor(FingerprintStore, { name: "FingerprintStore" });
+		await system.ask(ref, FingerprintStoreMessages.StoreFingerprint(makeFingerprint()));
+		await system.ask(ref, FingerprintStoreMessages.Block("fp-1" as any));
+		const doc = await mockDbInstance.collection("fingerprints").findOne({ uid: "fp-1" });
+		expect((doc as any).blocked).toBe(true);
+	});
+
+	test("a failed fingerprint write surfaces to the error receiver instead of being swallowed", async () => {
+		await system.createActor(ErrorReceiver, { name: "ErrorActor", errorReceiver: true });
+		const ref = await system.createActor(FingerprintStore, { name: "FingerprintStore" });
+		vi.spyOn(mockDbInstance.collection("fingerprints"), "updateOne").mockRejectedValueOnce(
+			new Error("connection lost")
+		);
+		// Fire without awaiting: on throw the ask never resolves, but the error is surfaced.
+		void system.ask(ref, FingerprintStoreMessages.StoreFingerprint(makeFingerprint())).catch(() => undefined);
+		await waitFor(() => captured.length > 0);
+		expect(captured.length).toBeGreaterThan(0);
+		expect(JSON.stringify(captured[0])).toContain("connection lost");
+	});
+});

packages/backend/test/sessionStore.test.ts

@@ -0,0 +1,83 @@
+import { Actor, ActorRef, ActorSystem } from "ts-actors";
+import { DateTime } from "luxon";
+import { toTimestamp } from "itu-utils";
+import { vi } from "vitest";
+import { SessionStore } from "../src/actors/SessionStore";
+import { Session, SessionStoreMessages } from "@recapp/models";
+import { mockDbInstance } from "./setup/mongoMock";
+
+// ts-actors surfaces a thrown handler error to the registered error receiver (it does not
+// reject the ask). Capturing here lets us assert the awaited write's failure is surfaced.
+const captured: unknown[] = [];
+class ErrorReceiver extends Actor<unknown, unknown> {
+	public constructor(name: string, system: ActorSystem) {
+		super(name, system);
+	}
+	public async receive(_from: ActorRef, message: unknown): Promise<unknown> {
+		captured.push(message);
+		return undefined;
+	}
+}
+
+async function waitFor(pred: () => boolean, timeoutMs = 2000): Promise<void> {
+	const start = Date.now();
+	while (!pred() && Date.now() - start < timeoutMs) {
+		await new Promise(r => setTimeout(r, 10));
+	}
+}
+
+function makeSession(overrides: Partial<Session> = {}): Session {
+	const now = toTimestamp();
+	const future = toTimestamp(DateTime.utc().plus({ hours: 1 }));
+	return {
+		idToken: "id",
+		accessToken: "access",
+		refreshToken: "refresh",
+		uid: "user-1" as any,
+		idExpires: future,
+		refreshExpires: future,
+		actorSystem: "client-system-1",
+		role: "STUDENT",
+		created: now,
+		updated: now,
+		...overrides,
+	};
+}
+
+// Characterises the await-and-call fix: SessionStore.StoreSession awaits storeEntity,
+// so the write is committed (and failures surfaced) before the ask resolves — previously
+// it was fire-and-forget, so a failed write during a Mongo blip was silently lost.
+describe("SessionStore — awaited persistence", () => {
+	let system: ActorSystem;
+
+	beforeEach(async () => {
+		captured.length = 0;
+		system = await ActorSystem.create({ systemName: "recapp-backend" });
+	});
+
+	afterEach(async () => {
+		await system.shutdown();
+		vi.restoreAllMocks();
+	});
+
+	test("StoreSession has committed the session to the DB by the time the ask resolves", async () => {
+		const ref = await system.createActor(SessionStore, { name: "SessionStore" });
+		await system.ask(ref, SessionStoreMessages.StoreSession(makeSession()));
+		const doc = await mockDbInstance.collection("sessions").findOne({ uid: "user-1" });
+		expect(doc).not.toBeNull();
+		expect((doc as any).accessToken).toBe("access");
+	});
+
+	test("a failed DB write surfaces to the error receiver instead of being swallowed", async () => {
+		await system.createActor(ErrorReceiver, { name: "ErrorActor", errorReceiver: true });
+		const ref = await system.createActor(SessionStore, { name: "SessionStore" });
+		vi.spyOn(mockDbInstance.collection("sessions"), "updateOne").mockRejectedValueOnce(
+			new Error("connection lost")
+		);
+		// Fire without awaiting: on throw the ask never resolves, but the error is surfaced.
+		void system.ask(ref, SessionStoreMessages.StoreSession(makeSession())).catch(() => undefined);
+		await waitFor(() => captured.length > 0);
+		expect(captured.length).toBeGreaterThan(0);
+		expect(JSON.stringify(captured[0])).toContain("connection lost");
+	});
+});