Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] External file updates lost during Capture prompt #210

Open
LER0ever opened this issue Mar 3, 2022 · 1 comment
Open

[BUG] External file updates lost during Capture prompt #210

LER0ever opened this issue Mar 3, 2022 · 1 comment
Assignees
Labels

Comments

@LER0ever
Copy link

@LER0ever LER0ever commented Mar 3, 2022

Describe the bug

Quickadd currently performs a full read of the target file upon command activation, and prompts for user input, and finally formats the input to target file. During the wait for input stage, which typically takes several seconds for user to type something, any external updates (typically from Obsidian Sync) to that file is lost. Quickadd then overwrites the file with updated ones based on the old version.

This is a followup to a bug report to Obsidian Sync that I submitted earlier to the Obsidian Forum. In that thread, @lishid pointed out that it could be one of the plugins overwriting the file with outdated content, which made me realize every time that Sync bug happens I was always using Quickadd to write something.

Cause

Current function call stack when triggering the Quickadd capture is as follows:

src/engine/CaptureChoiceEngine.ts: async run(): Promise<void>

const filePath = await this.getFilePath(captureTo);
let content = await this.getCaptureContent();
let file: TFile;

if (await this.fileExists(filePath)) {
    file = await this.getFileByPath(filePath);
    if (!file) return;

    const fileContent: string = await this.app.vault.read(file);
    const newFileContent: string = await this.formatter.formatContentWithFile(content, this.choice, fileContent, file);
    // the above function waits for user input. 
    await this.app.vault.modify(file, newFileContent);
}

And the this.formatter.formatContentWithFile would call into promptForValue() down the road, which creates an InputPrompt and waits for user input. Depending on how much text the user want to type, this would take several seconds to several minutes, and we definitely don't want to discard external updates to the file during this period.

A potential improvement

To avoid race condition, and minimize the possibility of simultaneous update to the same file, we'd like to minimize the duration between file reads and file writes, i.e. we'd like to make sure the update we perform is based on the latest version.

Current call sequence

  • read file content
  • format file content
    • prompt and wait for user input (long running)
    • process user input and format to file
    • return updated file
  • write updated file to vault

Suggested call sequence

  • prompt and wait for user input (long running)
  • read file content
  • format file content with user input
  • write updated file to vault

To Reproduce

The common use case to trigger this issue:

  1. iOS and Mac has note xxx.md at version 1
  2. Edit note (xxx.md) on Mac to make it version 2, Sync uploads the file
  3. Open iOS Obsidian
  4. Obsidian Sync tries to pull updates (version 2) from server
  5. Start QuickAdd capture from command, and start to write, QuickAdd reads file at version 1
  6. Obsidian Sync finishes the download, and updates the file xxx.md to version 2
  7. Finishes writing, click OK on QuickAdd prompt
  8. QuickAdd writes the formatted / updated file to vault, making it a new version (it's version 3 to Sync)
  9. The version 3 is synced across all device, version 2 update is lost

This is 100% consistently reproducible.

Expected behavior

If the file we want to edit is no longer the latest version, it should at least trigger the obsidian's conflict resolution, and should never nuke any file updates.

Additional context

Relevant Obsidian Sync Bug Report: https://forum.obsidian.md/t/obsidian-sync-updates-from-one-device-overwritten-by-another/33007

@lishid
Copy link

@lishid lishid commented Mar 4, 2022

That's such a great bug report!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants