From 9d99a4ac237ca3a081ecbcda63741c98049cb5d4 Mon Sep 17 00:00:00 2001 From: Andras Schmelczer Date: Fri, 8 May 2026 21:36:29 +0100 Subject: [PATCH] split: sync-client utils and errors reorganization Move error classes from services/ and file-operations/ into a new errors/ directory (authentication-error, server-version-mismatch-error, sync-reset-error, file-not-found-error), plus add file-already-exists-error and http-client-error. Update consts.ts and utils/* (await-all, create-client-id, hash, rate-limit, find-matching-file). Replace data-structures (locks, min-covered, event-listeners, fix-sized-cache) and add debugging utilities (in-memory-file-system, log-to-console, slow-web-socket-factory). Removes utils/create-promise.ts. --- frontend/sync-client/src/consts.ts | 6 +- .../authentication-error.ts | 0 .../src/errors/file-already-exists-error.ts | 9 + .../file-not-found-error.ts | 0 .../src/errors/http-client-error.ts | 9 + .../server-version-mismatch-error.ts | 0 .../{services => errors}/sync-reset-error.ts | 0 frontend/sync-client/src/utils/await-all.ts | 2 +- .../sync-client/src/utils/create-client-id.ts | 8 +- .../sync-client/src/utils/create-promise.ts | 25 --- .../utils/data-structures/event-listeners.ts | 74 +++++---- .../utils/data-structures/fix-sized-cache.ts | 2 +- .../src/utils/data-structures/locks.test.ts | 83 ++++++++-- .../src/utils/data-structures/locks.ts | 156 ++++++++++-------- .../utils/data-structures/min-covered.test.ts | 40 ++--- .../src/utils/data-structures/min-covered.ts | 15 +- .../utils/debugging/in-memory-file-system.ts | 69 ++++++++ .../src/utils/debugging/log-to-console.ts | 44 ++++- .../debugging/slow-web-socket-factory.ts | 2 +- .../src/utils/find-matching-file.ts | 13 +- frontend/sync-client/src/utils/hash.ts | 22 +-- frontend/sync-client/src/utils/rate-limit.ts | 20 +-- 22 files changed, 384 insertions(+), 215 deletions(-) rename frontend/sync-client/src/{services => errors}/authentication-error.ts (100%) create mode 100644 frontend/sync-client/src/errors/file-already-exists-error.ts rename frontend/sync-client/src/{file-operations => errors}/file-not-found-error.ts (100%) create mode 100644 frontend/sync-client/src/errors/http-client-error.ts rename frontend/sync-client/src/{services => errors}/server-version-mismatch-error.ts (100%) rename frontend/sync-client/src/{services => errors}/sync-reset-error.ts (100%) delete mode 100644 frontend/sync-client/src/utils/create-promise.ts create mode 100644 frontend/sync-client/src/utils/debugging/in-memory-file-system.ts diff --git a/frontend/sync-client/src/consts.ts b/frontend/sync-client/src/consts.ts index da70ba47..86319fd7 100644 --- a/frontend/sync-client/src/consts.ts +++ b/frontend/sync-client/src/consts.ts @@ -1,6 +1,6 @@ export const TIMEOUT_FOR_MERGING_HISTORY_ENTRIES_IN_SECONDS = 60; -export const DIFF_CACHE_SIZE_MB = 2; export const MAX_LOG_MESSAGE_COUNT = 100000; export const MAX_HISTORY_ENTRY_COUNT = 5000; -export const SUPPORTED_API_VERSION = 2; -export const WEBSOCKET_DISCONNECT_TIMEOUT_IN_S = 10; +export const SUPPORTED_API_VERSION = 3; +export const WEBSOCKET_DISCONNECT_TIMEOUT_IN_SECONDS = 10; +export const WEBSOCKET_CONNECTION_TIMEOUT_IN_SECONDS = 10; diff --git a/frontend/sync-client/src/services/authentication-error.ts b/frontend/sync-client/src/errors/authentication-error.ts similarity index 100% rename from frontend/sync-client/src/services/authentication-error.ts rename to frontend/sync-client/src/errors/authentication-error.ts diff --git a/frontend/sync-client/src/errors/file-already-exists-error.ts b/frontend/sync-client/src/errors/file-already-exists-error.ts new file mode 100644 index 00000000..35f51a66 --- /dev/null +++ b/frontend/sync-client/src/errors/file-already-exists-error.ts @@ -0,0 +1,9 @@ +export class FileAlreadyExistsError extends Error { + public constructor( + message: string, + public readonly filePath: string + ) { + super(message); + this.name = "FileAlreadyExistsError"; + } +} diff --git a/frontend/sync-client/src/file-operations/file-not-found-error.ts b/frontend/sync-client/src/errors/file-not-found-error.ts similarity index 100% rename from frontend/sync-client/src/file-operations/file-not-found-error.ts rename to frontend/sync-client/src/errors/file-not-found-error.ts diff --git a/frontend/sync-client/src/errors/http-client-error.ts b/frontend/sync-client/src/errors/http-client-error.ts new file mode 100644 index 00000000..2475cf35 --- /dev/null +++ b/frontend/sync-client/src/errors/http-client-error.ts @@ -0,0 +1,9 @@ +export class HttpClientError extends Error { + public constructor( + public readonly statusCode: number, + message: string + ) { + super(message); + this.name = "HttpClientError"; + } +} diff --git a/frontend/sync-client/src/services/server-version-mismatch-error.ts b/frontend/sync-client/src/errors/server-version-mismatch-error.ts similarity index 100% rename from frontend/sync-client/src/services/server-version-mismatch-error.ts rename to frontend/sync-client/src/errors/server-version-mismatch-error.ts diff --git a/frontend/sync-client/src/services/sync-reset-error.ts b/frontend/sync-client/src/errors/sync-reset-error.ts similarity index 100% rename from frontend/sync-client/src/services/sync-reset-error.ts rename to frontend/sync-client/src/errors/sync-reset-error.ts diff --git a/frontend/sync-client/src/utils/await-all.ts b/frontend/sync-client/src/utils/await-all.ts index 9406a6b8..43e06ce6 100644 --- a/frontend/sync-client/src/utils/await-all.ts +++ b/frontend/sync-client/src/utils/await-all.ts @@ -9,7 +9,7 @@ type ResolvedTuple = { export const awaitAll = async ( promises: PromiseTuple ): Promise> => { - // eslint-disable-next-line no-restricted-properties + // eslint-disable-next-line no-restricted-properties, @typescript-eslint/await-thenable const result = await Promise.allSettled(promises); for (const res of result) { if (res.status === "rejected") { diff --git a/frontend/sync-client/src/utils/create-client-id.ts b/frontend/sync-client/src/utils/create-client-id.ts index cfa132da..03dc2ae9 100644 --- a/frontend/sync-client/src/utils/create-client-id.ts +++ b/frontend/sync-client/src/utils/create-client-id.ts @@ -1,5 +1,3 @@ -import { v4 as uuidv4 } from "uuid"; - export function createClientId(): string { // @ts-expect-error, injected by webpack const packageVersion = __CURRENT_VERSION__; // eslint-disable-line @@ -8,8 +6,8 @@ export function createClientId(): string { typeof navigator !== "undefined" ? navigator.platform // eslint-disable-line @typescript-eslint/no-deprecated : typeof process !== "undefined" - ? process.platform - : "unknown"; + ? process.platform + : "unknown"; - return `vault-link/${packageVersion} (${uuidv4()}; ${platform})`; + return `vault-link/${packageVersion} (${Math.round(Math.random() * 1e10)}; ${platform})`; } diff --git a/frontend/sync-client/src/utils/create-promise.ts b/frontend/sync-client/src/utils/create-promise.ts deleted file mode 100644 index a49196ee..00000000 --- a/frontend/sync-client/src/utils/create-promise.ts +++ /dev/null @@ -1,25 +0,0 @@ -type ResolveFunction = undefined extends T - ? (value?: T) => unknown - : (value: T) => unknown; - -/** - * A type-safe utility function to create a Promise with resolve and reject functions. - * @returns A tuple containing a Promise, a resolve function, and a reject function. - */ -export function createPromise(): [ - Promise, - ResolveFunction, - (error: unknown) => unknown -] { - let resolve: undefined | ResolveFunction = undefined; - let reject: undefined | ((error: unknown) => unknown) = undefined; - - const creationPromise = new Promise( - (resolve_, reject_) => - // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion - ((resolve = resolve_ as ResolveFunction), (reject = reject_)) - ); - - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - return [creationPromise, resolve!, reject!]; -} diff --git a/frontend/sync-client/src/utils/data-structures/event-listeners.ts b/frontend/sync-client/src/utils/data-structures/event-listeners.ts index e08ca65e..420e4e63 100644 --- a/frontend/sync-client/src/utils/data-structures/event-listeners.ts +++ b/frontend/sync-client/src/utils/data-structures/event-listeners.ts @@ -13,56 +13,64 @@ export class EventListeners any> { } /** - * Adds a new listener to the collection. - * - * @param listener The listener callback to add - * @returns An unsubscribe function that removes this listener when called - */ + * Adds a new listener to the collection. + * + * @param listener The listener callback to add + * @returns An unsubscribe function that removes this listener when called + */ public add(listener: TListener): () => void { this.listeners.push(listener); return () => this.remove(listener); } /** - * Removes a listener from the collection. - * - * @param listener The listener callback to remove - * @returns true if the listener was found and removed, false otherwise - */ + * Removes a listener from the collection. + * + * @param listener The listener callback to remove + * @returns true if the listener was found and removed, false otherwise + */ public remove(listener: TListener): boolean { return removeFromArray(this.listeners, listener); } /** - * Triggers all listeners synchronously with the provided arguments. - * Any returned promises are ignored. Use triggerAsync() to await them. - * - * @param args The arguments to pass to each listener - */ + * Triggers all listeners synchronously with the provided arguments. + * Any returned promises are ignored. Use triggerAsync() to await them. + * + * @param args The arguments to pass to each listener + */ public trigger(...args: Parameters): void { - this.listeners.forEach((listener) => { + const snapshot = this.listeners.slice(); + for (const listener of snapshot) { + // allow removing listeners during the trigger loop + if (!this.listeners.includes(listener)) { + continue; + } listener(...args); - }); + } } /** - * Triggers all listeners and awaits any promises they return. - * Synchronous listeners are called immediately, and any async listeners - * are awaited in parallel. - * - * @param args The arguments to pass to each listener - */ + * Triggers all listeners and awaits any promises they return. + * Synchronous listeners are called immediately, and any async listeners + * are awaited in parallel. + * + * @param args The arguments to pass to each listener + */ public async triggerAsync(...args: Parameters): Promise { - await awaitAll( - this.listeners - .map((listener) => { - // eslint-disable-next-line @typescript-eslint/no-unsafe-return - return listener(...args); - }) - .filter((result): result is Promise => { - return result instanceof Promise; - }) - ); + const snapshot = this.listeners.slice(); + const promises: Promise[] = []; + for (const listener of snapshot) { + if (!this.listeners.includes(listener)) { + continue; + } + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + const result = listener(...args); + if (result instanceof Promise) { + promises.push(result); + } + } + await awaitAll(promises); } public clear(): void { diff --git a/frontend/sync-client/src/utils/data-structures/fix-sized-cache.ts b/frontend/sync-client/src/utils/data-structures/fix-sized-cache.ts index 51ad41c1..44a71dc8 100644 --- a/frontend/sync-client/src/utils/data-structures/fix-sized-cache.ts +++ b/frontend/sync-client/src/utils/data-structures/fix-sized-cache.ts @@ -1,6 +1,6 @@ // Implements an in-memory fixed-size cache for document contents, -import type { VaultUpdateId } from "../../persistence/database"; +import type { VaultUpdateId } from "../../sync-operations/types"; // Doubly-linked list node for O(1) LRU operations class LRUNode { diff --git a/frontend/sync-client/src/utils/data-structures/locks.test.ts b/frontend/sync-client/src/utils/data-structures/locks.test.ts index 9beb867a..c98bda0b 100644 --- a/frontend/sync-client/src/utils/data-structures/locks.test.ts +++ b/frontend/sync-client/src/utils/data-structures/locks.test.ts @@ -1,22 +1,24 @@ import { describe, it, beforeEach } from "node:test"; import assert from "node:assert"; import { Logger } from "../../tracing/logger"; -import type { RelativePath } from "../../persistence/database"; +import type { RelativePath } from "../../sync-operations/types"; import { Locks } from "./locks"; import { awaitAll } from "../await-all"; import { sleep } from "../sleep"; -import { SyncResetError } from "../../services/sync-reset-error"; +import { SyncResetError } from "../../errors/sync-reset-error"; describe("withLock", () => { const testPath: RelativePath = "test/document/path"; const testPath2: RelativePath = "test/document/path2"; + const testPath3: RelativePath = "test/document/path3"; + const logger = new Logger(); // eslint-disable-next-line @typescript-eslint/init-declarations let locks: Locks; beforeEach(() => { - locks = new Locks(logger); + locks = new Locks("locks-test", logger); }); it("should execute function with single key lock", async () => { @@ -56,22 +58,32 @@ describe("withLock", () => { it("should sort multiple keys to prevent deadlocks", async () => { const executionOrder: string[] = []; - // Start two concurrent operations with keys in different orders - const promise1 = locks.withLock([testPath2, testPath], async () => { - executionOrder.push("operation1-start"); - await sleep(50); - executionOrder.push("operation1-end"); - return "result1"; - }); + await locks.waitForLock(testPath); - const promise2 = locks.withLock([testPath, testPath2], async () => { - executionOrder.push("operation2-start"); - await sleep(50); - executionOrder.push("operation2-end"); - return "result2"; - }); + const promise = awaitAll([ + locks.withLock([testPath2, testPath3, testPath], async () => { + executionOrder.push("operation1-start"); + executionOrder.push("operation1-end"); + return "result1"; + }), - const [result1, result2] = await awaitAll([promise1, promise2]); + locks.withLock([testPath3, testPath, testPath2], async () => { + executionOrder.push("operation2-start"); + executionOrder.push("operation2-end"); + return "result2"; + }) + ]); + + locks.unlock(testPath); + + const [result1, result2] = await Promise.race([ + promise, + new Promise((_, reject) => { + setTimeout(() => { + reject(new Error("Deadlock detected")); + }, 1000); + }) + ]); assert.strictEqual(result1, "result1"); assert.strictEqual(result2, "result2"); @@ -234,13 +246,14 @@ describe("withLock", () => { describe("reset", () => { const testPath: RelativePath = "test/document/path"; + const testPath2: RelativePath = "test/document/path2"; const logger = new Logger(); // eslint-disable-next-line @typescript-eslint/init-declarations let locks: Locks; beforeEach(() => { - locks = new Locks(logger); + locks = new Locks("locks-test", logger); }); it("should reject pending waiters with SyncResetError while running operation completes", async () => { @@ -289,4 +302,38 @@ describe("reset", () => { const result = await locks.withLock(testPath, () => "success"); assert.strictEqual(result, "success"); }); + + it("should release partially acquired locks when reset interrupts multi-key acquisition", async () => { + // Hold testPath2 so multi-key acquisition will block on it + await locks.waitForLock(testPath2); + + // Start multi-key lock that will acquire testPath first, then block on testPath2 + const multiKeyPromise = locks.withLock( + [testPath, testPath2], + async () => "multi" + ); + void multiKeyPromise.catch(() => {}); // eslint-disable-line @typescript-eslint/no-empty-function + + // Wait for the multi-key operation to acquire testPath and start waiting on testPath2 + await sleep(10); + + // Reset should reject the waiting operation + locks.reset(); + + await assert.rejects(multiKeyPromise, (err: Error) => { + assert.ok(err instanceof SyncResetError); + return true; + }); + + // The key that was already acquired (testPath) should now be released + // This would hang/timeout if the lock was leaked + const result = await Promise.race([ + locks.withLock(testPath, () => "success"), + sleep(100).then(() => { + throw new Error("Lock was not released - deadlock detected"); + }) + ]); + + assert.strictEqual(result, "success"); + }); }); diff --git a/frontend/sync-client/src/utils/data-structures/locks.ts b/frontend/sync-client/src/utils/data-structures/locks.ts index e55c76b0..99c33075 100644 --- a/frontend/sync-client/src/utils/data-structures/locks.ts +++ b/frontend/sync-client/src/utils/data-structures/locks.ts @@ -1,6 +1,5 @@ -import { SyncResetError } from "../../services/sync-reset-error"; +import { SyncResetError } from "../../errors/sync-reset-error"; import type { Logger } from "../../tracing/logger"; -import { awaitAll } from "../await-all"; /** * Manages exclusive locks on items to prevent concurrent modifications. @@ -8,47 +7,53 @@ import { awaitAll } from "../await-all"; * * @template T The type of the key used for locking */ +/** Waiter entry with callbacks */ +interface WaiterEntry { + resolve: () => unknown; + reject: (err: unknown) => unknown; +} + export class Locks { /** Currently locked keys */ private readonly locked = new Set(); - /** Queue of resolve functions waiting for each key */ - private readonly waiters = new Map< - T, - [() => unknown, (err: unknown) => unknown][] - >(); + /** Queue of waiters for each key */ + private readonly waiters = new Map(); - public constructor(private readonly logger?: Logger) {} + public constructor( + private readonly name: string, + private readonly logger?: Logger + ) {} /** - * Executes a function while holding exclusive locks on one or more keys. - * - * This method ensures that the provided function runs with exclusive access to the - * specified key(s). Multiple keys are sorted to prevent deadlocks when different - * operations request the same keys in different orders. - * - * @template R The return type of the function to execute - * @param keyOrKeys A single key or array of keys to lock during function execution - * @param fn The function to execute while holding the lock(s). Can be sync or async. - * @returns A Promise that resolves to the return value of the executed function - * - * @example - * ```typescript - * // Lock a single key - * const result = await locks.withLock('file1', () => { - * // Critical section - only one operation can access 'file1' at a time - * return processFile('file1'); - * }); - * - * // Lock multiple keys (prevents deadlocks through consistent ordering) - * await locks.withLock(['file1', 'file2'], async () => { - * // Critical section - exclusive access to both files - * await moveFile('file1', 'file2'); - * }); - * ``` - * - * @throws Any error thrown by the provided function will be propagated after locks are released - */ + * Executes a function while holding exclusive locks on one or more keys. + * + * This method ensures that the provided function runs with exclusive access to the + * specified key(s). Multiple keys are sorted to prevent deadlocks when different + * operations request the same keys in different orders. + * + * @template R The return type of the function to execute + * @param keyOrKeys A single key or array of keys to lock during function execution + * @param fn The function to execute while holding the lock(s). Can be sync or async. + * @returns A Promise that resolves to the return value of the executed function + * + * @example + * ```typescript + * // Lock a single key + * const result = await locks.withLock('file1', () => { + * // Critical section - only one operation can access 'file1' at a time + * return processFile('file1'); + * }); + * + * // Lock multiple keys (prevents deadlocks through consistent ordering) + * await locks.withLock(['file1', 'file2'], async () => { + * // Critical section - exclusive access to both files + * await moveFile('file1', 'file2'); + * }); + * ``` + * + * @throws Any error thrown by the provided function will be propagated after locks are released + */ public async withLock( keyOrKeys: T | T[], fn: () => R | Promise @@ -59,12 +64,17 @@ export class Locks { const uniqueKeys = Array.from(new Set(keys)); uniqueKeys.sort((a, b) => String(a).localeCompare(String(b))); // Ensure consistent order to prevent deadlocks - await awaitAll(uniqueKeys.map(async (key) => this.waitForLock(key))); - + const lockedKeys = []; try { + for (const key of uniqueKeys) { + // Must acquire locks in-order (not concurrently) to prevent deadlocks + await this.waitForLock(key); + lockedKeys.push(key); + } + return await fn(); } finally { - uniqueKeys.forEach((key) => { + lockedKeys.forEach((key) => { this.unlock(key); }); } @@ -74,7 +84,7 @@ export class Locks { // Resolve all waiting promises before clearing to prevent deadlock // Any operation waiting for a lock will be granted access immediately for (const waiting of this.waiters.values()) { - for (const [_, reject] of waiting) { + for (const { reject } of waiting) { reject(new SyncResetError()); } } @@ -82,13 +92,17 @@ export class Locks { this.waiters.clear(); } + public isLocked(key: T): boolean { + return this.locked.has(key); + } + /** - * Attempts to acquire a lock immediately without waiting. - * Must call `unlock()` if successful. - * - * @param key The key to lock - * @returns `true` if lock acquired, `false` if already locked - */ + * Attempts to acquire a lock immediately without waiting. + * Must call `unlock()` if successful. + * + * @param key The key to lock + * @returns `true` if lock acquired, `false` if already locked + */ public tryLock(key: T): boolean { if (this.locked.has(key)) { return false; @@ -100,18 +114,18 @@ export class Locks { } /** - * Waits to acquire a lock, blocking until available. - * Operations are queued in FIFO order. Must call `unlock()` when done. - * - * @param key The key to wait for and lock - * @returns Promise that resolves when lock is acquired - */ + * Waits to acquire a lock, blocking until available. + * Operations are queued in FIFO order. Must call `unlock()` when done. + * + * @param key The key to wait for and lock + * @returns Promise that resolves when lock is acquired + */ public async waitForLock(key: T): Promise { if (this.tryLock(key)) { return Promise.resolve(); } - this.logger?.debug(`Waiting for lock on ${key}`); + this.logger?.debug(`Waiting for lock '${this.name}' on '${key}'`); return new Promise((resolve, reject) => { // DefaultDict behavior @@ -121,28 +135,36 @@ export class Locks { this.waiters.set(key, waiting); } - waiting.push([resolve, reject]); + waiting.push({ + resolve, + reject + }); }); } /** - * Releases a lock and grants access to the next waiting operation in FIFO order. - * Removes the key from locked set if no waiters. - * - * @param key The key to unlock - * @throws {Error} If key is not currently locked - */ + * Releases a lock and grants access to the next waiting operation in FIFO order. + * Removes the key from locked set if no waiters. + * + * @param key The key to unlock + * @throws {Error} If key is not currently locked + */ public unlock(key: T): void { if (!this.locked.has(key)) { + this.logger?.debug( + `Attempted to unlock '${this.name}' on '${key}' which is not locked` + ); return; } - // Remove first waiter to ensure FIFO order - const [resolveNextWaiting, _] = this.waiters.get(key)?.shift() ?? []; + this.logger?.debug(`Releasing lock '${this.name}' on '${key}'`); - if (resolveNextWaiting) { - this.logger?.debug(`Granted lock on ${key}`); - resolveNextWaiting(); + // Remove first waiter to ensure FIFO order + const nextWaiter = this.waiters.get(key)?.shift(); + + if (nextWaiter) { + this.logger?.debug(`Granted lock '${this.name}' on '${key}'`); + nextWaiter.resolve(); } else { this.locked.delete(key); } @@ -152,8 +174,8 @@ export class Locks { export class Lock { private readonly locks: Locks; - public constructor(logger?: Logger) { - this.locks = new Locks(logger); + public constructor(name: string, logger?: Logger) { + this.locks = new Locks(name, logger); } public async withLock(fn: () => R | Promise): Promise { diff --git a/frontend/sync-client/src/utils/data-structures/min-covered.test.ts b/frontend/sync-client/src/utils/data-structures/min-covered.test.ts index 7b7271d7..752227c0 100644 --- a/frontend/sync-client/src/utils/data-structures/min-covered.test.ts +++ b/frontend/sync-client/src/utils/data-structures/min-covered.test.ts @@ -1,15 +1,15 @@ import { describe, it } from "node:test"; import assert from "node:assert"; -import { CoveredValues } from "./min-covered"; +import { MinCovered } from "./min-covered"; -describe("CoveredValues", () => { +describe("MinCovered", () => { it("should initialize with the given min value", () => { - const covered = new CoveredValues(5); + const covered = new MinCovered(5); assert.strictEqual(covered.min, 5); }); it("should add values greater than min", () => { - const covered = new CoveredValues(0); + const covered = new MinCovered(0); covered.add(3); assert.strictEqual(covered.min, 0); covered.add(1); @@ -21,7 +21,7 @@ describe("CoveredValues", () => { }); it("should ignore duplicate values", () => { - const covered = new CoveredValues(0); + const covered = new MinCovered(0); covered.add(3); covered.add(3); covered.add(3); @@ -32,7 +32,7 @@ describe("CoveredValues", () => { }); it("should handle multiple consecutive values", () => { - const covered = new CoveredValues(132); + const covered = new MinCovered(132); for (let i = 250; i > 132; i--) { assert.strictEqual(covered.min, 132); covered.add(i); @@ -41,36 +41,32 @@ describe("CoveredValues", () => { }); it("should handle adding values lower than current min", () => { - const covered = new CoveredValues(5); + const covered = new MinCovered(5); covered.add(3); assert.strictEqual(covered.min, 5); covered.add(6); assert.strictEqual(covered.min, 6); }); - it("should auto-advance when setting min value", () => { - const covered = new CoveredValues(5); + it("should auto-advance when adding the value that fills the next gap", () => { + const covered = new MinCovered(5); covered.add(7); covered.add(8); covered.add(9); assert.strictEqual(covered.min, 5); - // Setting min to 6 should auto-advance through 7, 8, 9 - covered.min = 6; + // Adding 6 fills the gap and auto-advances through 7, 8, 9 + covered.add(6); assert.strictEqual(covered.min, 9); covered.add(10); assert.strictEqual(covered.min, 10); }); - it("should handle setting min value with no consecutive values", () => { - const covered = new CoveredValues(5); - covered.add(10); - covered.add(15); - assert.strictEqual(covered.min, 5); - // Setting min to 8 should not auto-advance (no consecutive values) - covered.min = 8; - assert.strictEqual(covered.min, 8); - // Add 9 to trigger auto-advance to 10 - covered.add(9); - assert.strictEqual(covered.min, 10); + it("should rewind when reset is called explicitly", () => { + const covered = new MinCovered(5); + covered.add(7); + covered.reset(3); + assert.strictEqual(covered.min, 3); + covered.add(4); + assert.strictEqual(covered.min, 4); }); }); diff --git a/frontend/sync-client/src/utils/data-structures/min-covered.ts b/frontend/sync-client/src/utils/data-structures/min-covered.ts index 8b38822f..ed0b9d2e 100644 --- a/frontend/sync-client/src/utils/data-structures/min-covered.ts +++ b/frontend/sync-client/src/utils/data-structures/min-covered.ts @@ -7,13 +7,13 @@ * * @example * ```typescript - * const covered = new CoveredValues(0); + * const covered = new MinCovered(0); * covered.add(2); // seenValues = [2], min = 0 * covered.add(1); // seenValues = [], min = 2 * covered.min; // returns 2 * ``` */ -export class CoveredValues { +export class MinCovered { private seenValues: number[] = []; public constructor(private minValue: number) {} @@ -22,12 +22,6 @@ export class CoveredValues { return this.minValue; } - public set min(value: number) { - this.minValue = Math.max(value, this.minValue); - this.seenValues = this.seenValues.filter((v) => v > this.minValue); - this.advanceMinWhilePossible(); - } - public add(value: number | undefined): void { if (value === undefined || value < this.minValue) { return; @@ -49,6 +43,11 @@ export class CoveredValues { this.advanceMinWhilePossible(); } + public reset(minValue?: number): void { + this.minValue = minValue ?? 0; + this.seenValues = []; + } + private advanceMinWhilePossible(): void { while ( this.seenValues.length > 0 && diff --git a/frontend/sync-client/src/utils/debugging/in-memory-file-system.ts b/frontend/sync-client/src/utils/debugging/in-memory-file-system.ts new file mode 100644 index 00000000..0d26b175 --- /dev/null +++ b/frontend/sync-client/src/utils/debugging/in-memory-file-system.ts @@ -0,0 +1,69 @@ +import type { RelativePath } from "../../sync-operations/types"; +import type { TextWithCursors } from "reconcile-text"; +import type { FileSystemOperations } from "../../file-operations/filesystem-operations"; + +export class InMemoryFileSystem implements FileSystemOperations { + protected readonly files = new Map(); + + public async listFilesRecursively( + _root: RelativePath | undefined = undefined // we don't use multi-level paths during tests + ): Promise { + return Array.from(this.files.keys()); + } + + public async read(path: RelativePath): Promise { + const file = this.files.get(path); + if (!file) { + throw new Error(`File ${path} does not exist`); + } + return file; + } + + public async write(path: RelativePath, content: Uint8Array): Promise { + this.files.set(path, content); + } + + public async atomicUpdateText( + path: RelativePath, + updater: (current: TextWithCursors) => TextWithCursors + ): Promise { + const file = this.files.get(path); + if (!file) { + throw new Error(`File ${path} does not exist`); + } + const currentContent = new TextDecoder().decode(file); + const newContent = updater({ text: currentContent, cursors: [] }).text; + this.files.set(path, new TextEncoder().encode(newContent)); + return newContent; + } + + public async getFileSize(path: RelativePath): Promise { + return (await this.read(path)).length; + } + + public async exists(path: RelativePath): Promise { + return this.files.has(path); + } + + public async createDirectory(_path: RelativePath): Promise { + // This doesn't mean anything in our virtual FS representation + } + + public async delete(path: RelativePath): Promise { + this.files.delete(path); + } + + public async rename( + oldPath: RelativePath, + newPath: RelativePath + ): Promise { + const file = this.files.get(oldPath); + if (!file) { + throw new Error(`File ${oldPath} does not exist`); + } + this.files.set(newPath, file); + if (oldPath !== newPath) { + this.files.delete(oldPath); + } + } +} diff --git a/frontend/sync-client/src/utils/debugging/log-to-console.ts b/frontend/sync-client/src/utils/debugging/log-to-console.ts index c47f18f6..def71400 100644 --- a/frontend/sync-client/src/utils/debugging/log-to-console.ts +++ b/frontend/sync-client/src/utils/debugging/log-to-console.ts @@ -1,10 +1,44 @@ -import type { SyncClient } from "../../sync-client"; -import type { LogLine } from "../../tracing/logger"; +/* eslint-disable no-console */ +import type { Logger, LogLine } from "../../tracing/logger"; import { LogLevel } from "../../tracing/logger"; -export function logToConsole(client: SyncClient): void { - client.logger.onLogEmitted.add((logLine: LogLine) => { - const formatted = `${logLine.timestamp.toISOString()} ${logLine.level} ${logLine.message}`; +const COLORS = { + reset: "\x1b[0m", + red: "\x1b[31m", + yellow: "\x1b[33m", + blue: "\x1b[34m", + gray: "\x1b[90m" +}; + +export function logToConsole( + logger: Logger, + { useColors = true }: { useColors?: boolean } = {} +): void { + logger.onLogEmitted.add((logLine: LogLine) => { + const timestamp = logLine.timestamp.toISOString(); + const { message } = logLine; + + let color = ""; + let reset = ""; + if (useColors) { + ({ reset } = COLORS); + switch (logLine.level) { + case LogLevel.ERROR: + color = COLORS.red; + break; + case LogLevel.WARNING: + color = COLORS.yellow; + break; + case LogLevel.INFO: + color = COLORS.blue; + break; + case LogLevel.DEBUG: + color = COLORS.gray; + break; + } + } + + const formatted = `${timestamp} ${color}${logLine.level}${reset} ${message}`; switch (logLine.level) { case LogLevel.ERROR: diff --git a/frontend/sync-client/src/utils/debugging/slow-web-socket-factory.ts b/frontend/sync-client/src/utils/debugging/slow-web-socket-factory.ts index c64bff18..b93460b5 100644 --- a/frontend/sync-client/src/utils/debugging/slow-web-socket-factory.ts +++ b/frontend/sync-client/src/utils/debugging/slow-web-socket-factory.ts @@ -11,7 +11,7 @@ export function slowWebSocketFactory( private static readonly RECEIVE_KEY = "websocket-receive"; private static readonly SEND_KEY = "websocket-send"; - private readonly locks = new Locks(logger); + private readonly locks = new Locks(FlakyWebSocket.name, logger); public set onopen(callback: ((event: Event) => void) | null) { super.onopen = async (event: Event): Promise => { diff --git a/frontend/sync-client/src/utils/find-matching-file.ts b/frontend/sync-client/src/utils/find-matching-file.ts index c3d323d3..1e0b352c 100644 --- a/frontend/sync-client/src/utils/find-matching-file.ts +++ b/frontend/sync-client/src/utils/find-matching-file.ts @@ -1,14 +1,17 @@ -import type { DocumentRecord } from "../persistence/database"; +import type { DocumentRecord } from "../sync-operations/types"; import { EMPTY_HASH } from "./hash"; // TODO: make this smarter so that offline files can be renamed & edited at the same time -export function findMatchingFile( +export async function findMatchingFile( contentHash: string, candidates: DocumentRecord[] -): DocumentRecord | undefined { - if (contentHash === EMPTY_HASH) { +): Promise { + if (contentHash === (await EMPTY_HASH)) { return undefined; } - return candidates.find(({ metadata }) => metadata?.hash === contentHash); + return candidates.find( + (record) => + record.remoteHash !== undefined && record.remoteHash === contentHash + ); } diff --git a/frontend/sync-client/src/utils/hash.ts b/frontend/sync-client/src/utils/hash.ts index 906b6fad..b9d23041 100644 --- a/frontend/sync-client/src/utils/hash.ts +++ b/frontend/sync-client/src/utils/hash.ts @@ -1,12 +1,14 @@ -// https://stackoverflow.com/questions/7616461/generate-a-hash-from-string-in-javascript -export function hash(content: Uint8Array): string { - let result = 0; - // eslint-disable-next-line @typescript-eslint/prefer-for-of - for (let i = 0; i < content.length; i++) { - result = (result << 5) - result + content[i]; - result |= 0; // Convert to 32bit integer - } - return Math.abs(result).toString(16).padStart(8, "0"); +export async function hash(content: Uint8Array): Promise { + // Re-wrap into a fresh Uint8Array so SubtleCrypto's + // BufferSource overload accepts it without an unsafe type assertion. + // The lib types require an ArrayBuffer-backed view; the source may + // be backed by SharedArrayBuffer in some runtimes. + const buffer = new ArrayBuffer(content.byteLength); + new Uint8Array(buffer).set(content); + const digest = await crypto.subtle.digest("SHA-256", buffer); + const bytes = new Uint8Array(digest); + return Array.from(bytes, (b) => b.toString(16).padStart(2, "0")).join(""); } -export const EMPTY_HASH = hash(new Uint8Array(0)); +// SHA-256 of empty content, computed once at import time +export const EMPTY_HASH: Promise = hash(new Uint8Array()); diff --git a/frontend/sync-client/src/utils/rate-limit.ts b/frontend/sync-client/src/utils/rate-limit.ts index 52cbbce7..acb86393 100644 --- a/frontend/sync-client/src/utils/rate-limit.ts +++ b/frontend/sync-client/src/utils/rate-limit.ts @@ -1,4 +1,4 @@ -import { createPromise } from "./create-promise"; +import { awaitAll } from "./await-all"; import { sleep } from "./sleep"; /** @@ -45,18 +45,16 @@ export function rateLimit< newArgs = undefined; } - const [promise, resolve] = createPromise(); - running = promise; - sleep( + // `running` must signal both "minimum interval has elapsed" *and* + // "fn() has finished" — otherwise an `fn` that takes longer than + // the interval would let a queued waiter fire a concurrent `fn` + const interval = typeof minIntervalMs === "function" ? minIntervalMs() - : minIntervalMs - ) - .then(resolve) - .catch(() => { - // sleep cannot fail - }); - return fn(...args); + : minIntervalMs; + const fnPromise = fn(...args); + running = awaitAll([fnPromise.catch(() => undefined), sleep(interval)]); + return fnPromise; }; return decoratedFn;