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

Data-loss #18

Closed
lishid opened this issue Jul 15, 2021 · 22 comments
Closed

Data-loss #18

lishid opened this issue Jul 15, 2021 · 22 comments

Comments

@lishid
Copy link

@lishid lishid commented Jul 15, 2021

Got a data-loss reported today related to this plugin: https://forum.obsidian.md/t/deleting-all-content-of-a-note/20905
Seems like several other issues in the repo with similar issues.

There's a flaw in this plugin that replaces the active markdown file with whatever file that was passed from the event, which could be a different file. Keep in mind that file events like rename, modify, and file-open can be fired on files that are not open (maybe a plugin changes them, or maybe they were changed on disk).

I would suggest performing all operations using the file's true contents using vault.read(file) and then subsequently vault.write(file, contents), instead of accessing it from the "active" MarkdownView, which possibly isn't the same file.

Also need to make sure that writing back the file doesn't trigger your event again, causing an infinite loop.

const view = this.app.workspace.getActiveViewOfType(MarkdownView);

Meanwhile, I will temporarily delist this plugin from the plugin repo until these issues are fixed to avoid users losing data.

dvcrn added a commit that referenced this issue Jul 16, 2021
To avoid trigger through other means such as external renaming

References #18
@dvcrn
Copy link
Owner

@dvcrn dvcrn commented Jul 16, 2021

I've added a guard to check view.file against the file that triggered the event to catch cases like this

@lishid
Copy link
Author

@lishid lishid commented Jul 16, 2021

Oh I forgot to mention in the last comment - highly urge to consider using view.editor instead of view.sourceMode.cmEditor. cmEditor only exists on the desktop client and does not exist on mobile.

@lishid
Copy link
Author

@lishid lishid commented Jul 16, 2021

Also, were you able to reproduce the issue from the user's video? I looked through the code here again but I couldn't figure out why it'd wipe the file clean even if it ran into that condition.

@dvcrn
Copy link
Owner

@dvcrn dvcrn commented Jul 16, 2021

I watched his video but I wasn't able to reproduce this either. This plugin doesn't do actions on the full doc content and only replaces 1 line at max

replaceLine(doc: CodeMirror.Doc, line: number, text: string) {
doc.replaceRange(
`${text}\n`,
{ line: line, ch: 0 },
{ line: line + 1, ch: 0 },
);

It's odd that in his video everything besides the heading gets wiped.

There is also this guard that short-circuits when nothing changed, like in his video (the heading stayed the same)

https://github.com/dvcrn/obsidian-filename-header-sync/blob/4758c416b32d7b450b08d788e465ecf607d4447a/main.ts#L185

Oh I forgot to mention in the last comment - highly urge to consider using view.editor instead of view.sourceMode.cmEditor. cmEditor only exists on the desktop client and does not exist on mobile.

Will update that

@lishid
Copy link
Author

@lishid lishid commented Jul 16, 2021

Yeah I wasn't 100% sure that it's this plugin alone - maybe some weird race condition with another plugin but it's really hard to tell. As far as I see the insertion/replacement of a single line should be fine and isn't supposed to wipe out a file.

dvcrn added a commit that referenced this issue Jul 16, 2021
@cocoonkid
Copy link

@cocoonkid cocoonkid commented Jul 16, 2021

Apparently the Plugin has been force disabled inside Obsidian because of reported issues.

I love it and already want to say thank you for providing something so helpful.
Can't wait for the fix :)

@dvcrn
Copy link
Owner

@dvcrn dvcrn commented Jul 16, 2021

The issue stated was likely not caused by this plugin. I have pushed a few bugfixes to harden things up and will be releasing a new version a bit later today 👍

@lishid
Copy link
Author

@lishid lishid commented Jul 16, 2021

It should be back sometimes tomorrow once the new version is available and we put it back up in the repo.

@yappen
Copy link

@yappen yappen commented Jul 16, 2021

I've definitely also had data loss linked to this plugin. I didn't report because I couldn't figure out the steps to trigger the issues, the only thing I noticed was that it occured particularly frequently when links to PDF files were involved. I would love it if there was a fix, this is a fantastically useful plugin.

For those who have lost data, you may find that your previous file still exists. Try a search on your vault folder for a keyword in the missing file using Agent Ransack or a similar tool. I was able to recover files like that, saved under unexpected names.

Edit: If I remember correctly, it seemed that I ended up with two files with the same title. Maybe that could explain everything getting "wiped" - it isn't really wiped, but a new file with the same name gets created, which only contains the heading. This has happened to me.

@dvcrn
Copy link
Owner

@dvcrn dvcrn commented Jul 16, 2021

Interesting, do you remember what you did when that happened? Just opened a file?

I patched a issue where a file would get renamed to empty name when the heading was set to "", and obsidian didn't display it anymore.

It could also be an external event trigger like external file rename but we added a check for that today too.

A faulty rename seems the most plausible

Trying to figure out just by looking at the code how this could happen

@yappen
Copy link

@yappen yappen commented Jul 16, 2021

These things happened about two months ago, after that I deactivated the plugin until I tried it again yesterday. I'm picking my brains but I was already confused by what exactly was happening then, so it's all a bit fuzzy.

Thinking about it more I suppose they were two different but perhaps related issues:

  • One was that I would click a link to a PDF file in one of my Obsidian notes, view it inside Obsidian, then go back to the previous note (maybe using ctrl + alt + left arrow) and that would then be renamed to the name of the PDF file that had just been opened. It was mainly annoying renaming the files back, and I vaguely remember that the rename back to the original title wasn't smooth, requiring me to manually fix links. I'm not sure anymore that I experienced data loss in those cases.
  • The other issue seems to be the creation of an empty file with the same name as a recently opened file. Like in the video, it also seemed to happen when clicking a link to said file. One moment I'm viewing the file, I click over to another file, then click the link back to the previous file and now suddenly it's empty. In a moment of desparation I deleted the empty file, and then the link pointed to the correct file with its content again, that's why I'm assuming it created an empty file. This happened a few times after activating the plugin and never happened after I deactivated it. That's all I can remember right now, maybe someone else can add in more details.

Unlike the person who shared the video, I never have a file in the left sidebar, I didn't even know this was possible. But I do use the sliding panes plugin.

Generally, it definitely seemed to a renaming issue, sometimes renaming current files (and the heading) to the file name of a file that had been open previously, sometimes creating a new empty file with the same name. My impression was also that it was a renaming issue. Do you have auto-updating of renamed links enabled when testing? Mine is/was enabled when this happened, don't know if that could have something to do with it.

I patched a issue where a file would get renamed to empty name when the heading was set to "", and obsidian didn't display it anymore.

Thanks, that happened to me once! That's the one I found again through a full-text search, I think my memory was mixing different issues there.

@dpnem
Copy link

@dpnem dpnem commented Jul 17, 2021

I wanted to let you know I had disappearing data as well, but . . .

  1. I am unsure whether it was this plug-in that created the issue
  2. I could never recreate the issue
  3. If I remember correctly, it may have had more to do with moving files in the sidebar from folder to folder.

Anyway, I wanted to say I love the plugin and if you need any testers for a new release, I'd be more than happy to assist.

@dvcrn
Copy link
Owner

@dvcrn dvcrn commented Jul 19, 2021

If I remember correctly, it may have had more to do with moving files in the sidebar from folder to folder.

This would fit with the theory that an external event triggered this plugin. The guard I've added in the commit above should take care of this

I've created 1.4.0 with those fixes and support for skipping frontmatter - https://github.com/dvcrn/obsidian-filename-heading-sync/releases/tag/1.4.0

I'll try a few more things with the descriptions above but wanted to get those bugfixes out first

@cocoonkid
Copy link

@cocoonkid cocoonkid commented Jul 19, 2021

❤️

@setu4993
Copy link

@setu4993 setu4993 commented Aug 15, 2021

This just happened to me on v1.4.2.

@dvcrn
Copy link
Owner

@dvcrn dvcrn commented Aug 17, 2021

@setu4993 do you remember what you did to cause this? Did the file get renamed to a file without name like in previous cases?

@setu4993
Copy link

@setu4993 setu4993 commented Aug 18, 2021

@dvcrn : Thanks for checking in. It was similar to the video linked out in the forum post in the OP. I'll try to reproduce and record in a couple days, but best I can recollect:

  1. Created a new note.
  2. Added a template to it, filled it.
  3. Added content.
  4. Linked to the note within itself (incorrectly).
  5. Command + click (MacOS) on the self-link to open the linked note (in this case, self).
  6. Same note opens, but is blank.

@dvcrn
Copy link
Owner

@dvcrn dvcrn commented Aug 18, 2021

Thanks for the steps!

Same note opens, but is blank.

Obsidian has this feature that you can link to notes that don't exist. If you click on one of those links, it will create a blank note at the link destination.

It sounds to me like this is what happened here: The note got renamed to something else but the link within the note not updated. When you clicked on the link it opened a note at the previous location which no longer exists because it got renamed, and then creates a new note in it's place.

Now the question why did it rename the note incorrectly. We even have a guard that checks on filename length and doesn't do anything in that case: https://github.com/dvcrn/obsidian-filename-heading-sync/blob/master/main.ts#L127-L138

Could you check in your directory if you can find the note? If you do, can you tell me what filename it has? Previously it could have been ".md" (no name, just extension), but we have a guard against that now.

@setu4993
Copy link

@setu4993 setu4993 commented Aug 20, 2021

@dvcrn : Nope, no ".md" or another file that contains a backup. I just reproduced it from scratch and led to the same result, no backed up file.

Screen.Recording.2021-08-19.at.9.33.33.PM.mov

@dvcrn
Copy link
Owner

@dvcrn dvcrn commented Aug 20, 2021

Thanks for the video! That helps a lot. I'll see if I can reproduce it here

@dvcrn
Copy link
Owner

@dvcrn dvcrn commented Aug 20, 2021

I was able to replicate the issue. This plugin has an event listener on file-open so when you open the file in a new pane by clicking on the link, this event triggers and passes all the checks because the new file is the old file.

I still don't know why it resulted in this, but I removed the file-open listener for now and it fixed this issue.

Closing this for now, please re-open if something strange happens again.

@setu4993
Copy link

@setu4993 setu4993 commented Aug 20, 2021

Thanks for the quick debug and fix, @dvcrn! Kudos on the plugin, I love it :).

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

Successfully merging a pull request may close this issue.

None yet
6 participants