fix(wa-sqlite): recover failed OPFS initialization - #230
Conversation
e6f7a42 to
2e31d27
Compare
|
|
||
| type LifecycleHarness = NonNullable<Window['__treecrdtLifecycle']>; | ||
| type LifecycleRuntime = 'direct' | 'dedicated-worker' | 'shared-worker'; | ||
| type LifecycleOptions = { |
There was a problem hiding this comment.
Is it a duplicate of the same type from packages/treecrdt-wa-sqlite/e2e/src/lifecycle.ts? Can it be reused?
There was a problem hiding this comment.
Goood catch fixed!
| fallback?: 'memory' | 'throw'; | ||
| filename: string; | ||
| runtime: LifecycleRuntime; | ||
| sharedWorkerName?: string; |
There was a problem hiding this comment.
Why is it necessary to provide the name of the shared worker?
There was a problem hiding this comment.
The explicit name is needed for these regression tests, where want it to be the same name to make sure stale port/session cleanup works. I added a comment
|
|
||
| const OPFS_VFS_NAME = 'opfs'; | ||
|
|
||
| function errorMessage(err: unknown): string { |
There was a problem hiding this comment.
We have so many such helper functions that create bloat in the code. Maybe let's try to remove them one by one
There was a problem hiding this comment.
Agree, fixed! See my latest commit
| const handle = await sqlite3.open_v2(filename, undefined, OPFS_VFS_NAME); | ||
| return { handle, vfs: initializedVfs }; | ||
| } catch (err) { | ||
| await closeIgnoringErrors(vfs?.close ? () => vfs!.close!() : undefined); |
There was a problem hiding this comment.
this also looks scary. Can we simplify it. Also, closeIgnoringErrors seems as another "helper" function. I would like to simplify such conditions vfs?.close ? () => vfs!.close!()
There was a problem hiding this comment.
Good catch.. I removed closeIgnoringErrors and the callback expressions. Cleanup is now performed directly with try/catch. The catch remains so a cleanup failure cannot replace the original initialization/open error. This keeps the same behavior with simpler code
2 participants
Before
fallback: "throw"was not forwarded into dedicated or SharedWorker initialization. If OPFS startup failed, the worker attempted memory initialization before the outer client rejected. Partial SQLite/VFS state and SharedWorker ports could remain, making later initialization unreliable.After
Workers now follow the same policy as direct clients:
throw: clean partial resources and reject without opening memory storage.memory: when the OPFS VFS or database cannot open, clean partial resources and retry using a fresh wa-sqlite module.TreeCRDT extension or document initialization failures still reject instead of silently replacing a persistent database with empty memory storage. SharedWorker initialization, close, and drop release both client- and worker-side port ownership. Successful OPFS startup is unchanged.
Impact on em
em uses OPFS with
fallback: "throw"in a dedicated worker. Storage startup failures will now reject cleanly and predictably, allowing em to handle the unsupported-storage state explicitly. This does not add OPFS support where the browser disables it, such as Safari Private Browsing, and em still owns the user-facing initialization error state.This is the main-based replacement for the OPFS recovery work from #208 and leaves the teardown behavior released through #229 unchanged.