Skip to content

fix: yield results from _enumerate when metaOnly is true#22

Open
es617 wants to merge 5 commits into
vrtmrz:mainfrom
es617:fix/enumerate-metaonly-yield-v2
Open

fix: yield results from _enumerate when metaOnly is true#22
es617 wants to merge 5 commits into
vrtmrz:mainfrom
es617:fix/enumerate-metaonly-yield-v2

Conversation

@es617

@es617 es617 commented Mar 26, 2026

Copy link
Copy Markdown

Resubmitted from #21 with a clean base on current main.

Problem

DirectFileManipulator._enumerate() silently returns zero results when called with metaOnly: true.

In an async * generator function, a bare return <value> doesn't yield anything to the caller — it just sets the generator's return value (which for await...of ignores). The current code returns the async iterator from findEntries() as a completion value instead of delegating to it.

// Before: returns the iterator as a value, yields nothing
async *_enumerate(startKey, endKey, opt) {
    if (opt.metaOnly) return this.liveSyncLocalDB.findEntries(startKey, endKey, {});
    // ...
}

This means enumerateAllNormalDocs({ metaOnly: true }) always produces an empty sequence.

Fix

// After: delegates to the iterator, yields all its values
if (opt.metaOnly) return yield* this.liveSyncLocalDB.findEntries(startKey, endKey, {});

yield* delegates to the inner async generator, forwarding all its values to the caller.

LiveSyncManagers.log() calls addLog during initialization, but the
handler was never set. Other handlers (saveData, loadData) are set
in the constructor — addLog was missed during the service refactor.
@es617

es617 commented Mar 26, 2026

Copy link
Copy Markdown
Author

Update: Found a second bug in the same file while testing.

Problem 2: addLog handler not set in DirectFileManipulator constructor

LiveSyncManagers.log() calls addLog during initialization, but the handler is never assigned. Other handlers (saveData, loadData, onDatabaseInitialisation) are set in the constructor — addLog was missed during the service refactor.

  Error: Handler addLog is not assigned.
      at Binder.invoke
      at HeadlessAPIService.func [as addLog]
      at LiveSyncManagers.log
      at LiveSyncManagers.getManagerMembers
      at new LiveSyncManagers
      at LiveSyncLocalDB.initializeDatabase

Fix: Set a no-op handler alongside the other handler assignments in the constructor:

(this.services.API as InjectableAPIService<ServiceContext>).addLog.setHandler(() => {});

Both fixes are in this PR (two commits).

- Set addLog handler on API service (prevents 'Handler addLog is not assigned')
- Set settings on setting service before init (prevents undefined settings in HashManager)
- Register LiveSyncLocalDB with database service (prevents 'Local database is not ready yet')

All three are caused by DirectFileManipulator bypassing the normal
DatabaseService.openDatabase() flow after the service architecture refactor.
@es617

es617 commented Mar 26, 2026

Copy link
Copy Markdown
Author

Update 2: Two more initialization bugs, same pattern as Problem 2.

Problem 3: Settings not initialized before LiveSyncManagers construction

HashManager calls settingService.currentSettings() during construction, but SettingService._settings is never populated. The old currentSettings binder (line 142-144) was commented out during the refactor without a replacement.

TypeError: Cannot read properties of undefined (reading 'encrypt')
    at HashManager.applyOptions
    at new HashManagerCore
    at LiveSyncManagers.getManagerMembers

Problem 4: LiveSyncLocalDB not registered with DatabaseService

LiveSyncManagers.getManagerMembers() accesses databaseService.localDatabase, but DirectFileManipulator creates LiveSyncLocalDB directly — bypassing DatabaseService.openDatabase() where _localDatabase normally gets set.

Error: Local database is not ready yet.
    at get localDatabase
    at LiveSyncManagers.getManagerMembers

Fix

Both fixed in the same commit as the addLog fix:

this.services.setting.settings = this.settings as any;
// ...after creating LiveSyncLocalDB:
(this.services.database as any)._localDatabase = this.liveSyncLocalDB;

All four fixes now in this PR (3 commits). Tested end-to-end against a live CouchDB vault with E2E encryption.

getByMeta can throw when decryption fails (wrong passphrase).
Previously this was an unhandled rejection that killed the process.
Now logs a warning and skips the document.
@es617

es617 commented Mar 26, 2026

Copy link
Copy Markdown
Author

sorry, this is growing as I am finding issues. Happy to break this down if needed.

Update 4: One more crash found during testing with encrypted vaults.

Problem 5: beginWatch crashes on decryption failure

When getByMeta() fails to decrypt a document (wrong passphrase, corrupted doc), the unhandled rejection in the _changes listener kills the Node process.

DOMException [OperationError]: The operation failed for an operation-specific reason
    at AESCipherJob.onDone (node:internal/crypto/util:453:19)

The _changes listener calls getByMeta() without try/catch:

const docX = await this.getByMeta(doc);  // throws on decrypt failure

Fix

Wrap getByMeta() in try/catch, log a warning, and skip the document:

let docX;
try {
    docX = await this.getByMeta(doc);
} catch (ex) {
    Logger(`WATCH: DECRYPT FAILED: ${doc.path}`, LEVEL_INFO, "watch");
    Logger(ex, LEVEL_VERBOSE, "watch");
    return;
}

All five fixes now in this PR (4 commits).

The service refactor moved path2id through PathService which reads
usePathObfuscation from settings. DirectFileManipulator's settings
getter never set this field, so path obfuscation was silently disabled.
The old code bypassed PathService and passed obfuscatePassphrase
directly to path2id_base.
@es617

es617 commented Mar 26, 2026

Copy link
Copy Markdown
Author

Update 5: Path obfuscation regression.

Problem 6: usePathObfuscation not set in settings getter

The old code passed obfuscatePassphrase directly to path2id_base:

// Old (worked):
async path2id(filename, prefix?) {
    return await path2id_base(fileName, this.options.obfuscatePassphrase ?? false, ...);
}

The service refactor routes path2id through PathService, which reads settings.usePathObfuscation:

// New PathService.path2id:
setting.usePathObfuscation ? setting.passphrase : ""

But DirectFileManipulator's settings getter never sets usePathObfuscation, so it defaults to false. This means:

  • Reads fail for docs with obfuscated (f:) IDs — path2id generates plain IDs, CouchDB returns 404
  • Writes create docs with plain IDs instead of f: IDs — invisible to clients expecting obfuscated IDs

Fix

Set usePathObfuscation from the existing obfuscatePassphrase option:

usePathObfuscation: !!this.options.obfuscatePassphrase,

All six fixes now in this PR (5 commits).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant