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]: Wrong timestamps in both local and remote, and subsequent DB problems #60

Open
1 of 2 tasks
Engineer-Extreme opened this issue Dec 7, 2023 · 20 comments
Open
1 of 2 tasks
Labels
bug Something isn't working

Comments

@Engineer-Extreme
Copy link

Engineer-Extreme commented Dec 7, 2023

What happened?

I think this might actually turn out to be multiple different issues.

Possible Issue 1:
When I sync things the first time, everything in the vault gets uploaded to the remote. Unfortunately, during this upload, all the files on remote acquire "current timestamp" as the modification time even though those files were last modified years ago on local. So, when the next time I trigger the sync, the remote timestamps are absolutely recent and everything gets redownloaded, thus changing the timestamp of all local files to "current timestamp".

Possible Issue 2:
When I copy my local vault to a new device, the file's "Created time" gets set to the "current timestamp" by the OS (I am using MacOS and an external USB flash drive with FAT file system). The modification time somehow remain correct here. But unfortunately, when Remotely Sync tries to read the local timestamps of the file, instead of reading the modification time, it somehow uses "the latest of the creation time and modification time", thus selecting the "current timestamp" creation time. This then uploads everything to the cloud again.

Issues 3 and 4 moved to #61 (@sboesen edit)
Possible Issue 3:
I tried to circumvent Possible Issue 1 by pasting the local vault with correct creation and modification times to OneDrive manually (which uploads correct modification times on the remote), but then when I tried to Remotely Sync, it somehow assumed that all the files were deleted in the cloud and hence deleted my entire local vault. I did a DB reset before testing every issue I identified above.

Possible Issue 4:
Which brings me to another issue regarding the database which stores metadata. Because my entire local vault was deleted, and that was the state of Remotely Sync even after multiple retries, I decided to reinstall the plugin, authenticate from scratch so that it would at least not keep that delete state for every file. I also deleted the remote "Apps/Remotely Secure" folder on OneDrive. But unfortunately, I don't know how but the "deletion state" still exists and Remotely Sync still assumes the files were all deleted in local. And this after reinstalling in the same vault without restoring those deleted files. Somehow Remotely Sync still has the listing of all of the original files even though there is nothing in the vault and I reset Remotely Sync. It should start with an empty file list with no history from before the reinstall, but it has the history of all of the files in the vault which actually don't exist anymore!

What OS are you using?

macOS

What remote cloud services are you using?

OneDrive for personal

Version of the plugin

0.4.25

Version of Obsidian

1.4.16

Using password or not

  • Yes.

Ensure no sensitive information

  • I ensure that no sensitive information is submitted in the issue.
@Engineer-Extreme Engineer-Extreme added the bug Something isn't working label Dec 7, 2023
@sboesen
Copy link
Owner

sboesen commented Dec 7, 2023

Thanks for filing. Definitely seen these types of issues myself, the root cause is we don't send any timestamp data to the remote server. As we surface more sync information through the status bar including for automatic syncs and add more automatic syncing triggers this becomes more of an obvious issue, and one worth fixing.

We have some improvements to the sync process tracked in #40. I'll keep this issue open and focused on sending modification time to the remote (we don't compare by create time currently, so at least modification time).

For OneDrive there is this property it looks like we can send for personal accounts.

lastAccessedDateTime is not available for items in SharePoint online or OneDrive for Business.

We can probably send these for other cloud providers, too, but moving beyond just modification date would be ideal, and syncing modification time locally is worth fixing too (vs. "I just downloaded this file from remote 2 seconds ago, set modification time to now, thus local file is newer than remote" triggering endless syncing)

@sboesen
Copy link
Owner

sboesen commented Dec 7, 2023

Moved potential issues 3/4 to #61.

@Engineer-Extreme
Copy link
Author

the root cause is we don't send any timestamp data to the remote server.

Yes, exactly. That's what my diagnosis was too. Once the files are synced up to remote the first time, they acquire the "current timestamp" as the modification time. After that, whenever once syncs the next time, everything syncs down because the cloud modification times are later than local modification times. Eventually every copy of the vault where I sync down, gets that "current timestamp".

We can probably send these for other cloud providers, too,

I was able to replicate the same problems in DropBox too.

we don't compare by create time currently, so at least modification time

Yes this is a good idea. I would actually suggest not using creation times at all since in many systems the creation time gets reset in many situations, such as if you make a copy of a file. I was able to see this particularly when I copied my vault over to a FAT file system drive where the creation time for every file got reset to "current timestamp". Using creation time might mess things up more, just sticking with modification time is better.

@sboesen
Copy link
Owner

sboesen commented Dec 8, 2023

Yep - the goal is to explore something like file hashes as well, maintaining a local + encrypted remote list of hashes for quick updating. Will need to compare approaches and likely will be a mix of mtime + hashes - we'll see. That will be explored in #40.

Keeping this issue focused on sending mtime to cloud providers, which is definitely a gap.

@sboesen
Copy link
Owner

sboesen commented Dec 12, 2023

Should be fixed in Dropbox and S3, webdav and OneDrive next.

@sboesen
Copy link
Owner

sboesen commented Dec 16, 2023

Looks like it's not as obvious how to add modification time to OneDrive (looks like they support it, but what parameter?) and Webdav + some s3 providers don't support setting this. Will take the time to add this via the metadata file (add each file as an entry with modification time) as a generic way of storing this for all remote providers.

@Engineer-Extreme
Copy link
Author

Finally had the time to test out the fix using Dropbox, but unfortunately it doesn't seem to have worked.

I started with a fresh vault, latest version of the plugin, and fresh Dropbox authentication. But this issue still seems to be happening. First time I synced from one copy of the vault, the modification times of the "first-time-sync-so-upload-everything" files was "the current timestamp i.e. time of sync". And then when in another copy of the vault I sync down from Dropbox, every file acquired the modification time as that previous "current timestamp".

This is the same as what was happening before.

@kadisonm
Copy link

kadisonm commented Jan 4, 2024

Might be related to what I found yesterday where dropbox's get modification function always returns undefined?

@sboesen
Copy link
Owner

sboesen commented Jan 4, 2024

Hmm. Sorry it's not working, but I think we need to replace this implementation with a generic solution anyway. Not sure why it isn't working for Dropbox (I thought I was able to get it working, but it's been a while and I can't remember how thorough I tested this).

Appreciate you taking the time to test it!

@kadisonm which function is this, could you link it?

@kadisonm
Copy link

kadisonm commented Jan 4, 2024

async function getDropboxMtimeString(vault: Vault, fileOrFolderPath: string) : Promise<string> {

It is used once but I haven't checked entirely what it's used for, it seemed like an unfinished function to get modification time but is being used.

const res = await getRemoteMeta(client, uploadFile);

https://github.com/sboesen/remotely-sync/blob/a43de9fc0060570c17321783ad69ba4c4bc0f5ea/src/remoteForDropbox.ts#L575C6-L575C6

The above is where it is used. You can see that it sets a client modified property to undefined (not sure if this is intended)

https://github.com/sboesen/remotely-sync/blob/a43de9fc0060570c17321783ad69ba4c4bc0f5ea/src/remoteForDropbox.ts#L456C19-L456C19

I also know this function does the same thing since it returns an object with a server_modified property. I think having a quick look this is used most the time.

@sboesen
Copy link
Owner

sboesen commented Jan 5, 2024

Thank you! Bah, clearly a mistake there. Sorry about that. Updated since it's a one-line change but still plan for the generic solution.

@Engineer-Extreme
Copy link
Author

Engineer-Extreme commented Jan 5, 2024

Just tried with the latest version of the plugin, and I am not sure why but no files are getting uploaded to the Dropbox remote. I see through the sync plans export that all files get "uploadLocalToRemote" as the decision, but they don't actually get uploaded. I am not sure why this would be happening though since the only change you made is w.r.t. the modification times.

Anyway, I also spotted another issue when browsing the sync plans export. The mTimeLocalFmt seems to be picking up the creation time of a file instead of the modification time. At least as I see on my Mac, if I copy a vault then the modification timestamps for the files get copied correctly, but the creation time gets reset to the time the files are copied. Hence why in my vault the creation time is very new because I just made a copy of my vault to experiment with the new plugin version, and that new timestamp is what I see get picked up in mTimeLocalFmt. This may be a separate issue though so feel free to create a new one.

@kadisonm
Copy link

kadisonm commented Jan 6, 2024

Odd mine is syncing fine. Is there anything in the console when you turn on debug mode?

@kadisonm
Copy link

kadisonm commented Jan 6, 2024

Okay so I tested on a vault with no metadata by deleting before and now I'm experiencing the same issue. No errors in the console.

@kadisonm
Copy link

kadisonm commented Jan 6, 2024

@sboesen think I fixed it. In getDateStringFromMtime() the ISO string being passed in is wrong. Fixed it by removing * 1000 in the new Date() since it can be instantiated with just milliseconds. Not sure why the error only shows in the console when developing.

https://github.com/kadisonm/remotely-sync/blob/f02a035b66365edeb51be8dcf5090191cac0eb4b/src/remoteForDropbox.ts#L499C10-L499C10

Made a PR so you don't have to load everything up just to make a one line change. I'll also just double check the client_modified time is correct on the remote and local.
:)

Edit: Seems to have worked. The modified times are correct on my Dropbox.

@sboesen
Copy link
Owner

sboesen commented Jan 7, 2024

Merged, thanks for the PR! releasing in 0.4.34 - hope that fixes it short term! Appreciate you testing/opening the PR :)

@Engineer-Extreme
Copy link
Author

Engineer-Extreme commented Jan 11, 2024

More testing, with fresh copies of the vault and latest version of the plugin:

Dropbox does show the correct "modified timestamps" of all the files now, but the issue is not fixed:

  1. Problem 1: When "syncing down" to a second copy of the vault with fresh auth, somehow the plugin is still picking up the upload time of the "sync up" to Dropbox as the modified time of the file. I verified this by checking the sync plans from the "sync down" and I can see that the mTimeRemote* fields have the time from today when I made the "sync up" from the first copy of the vault. I am not sure how this is happening because the individual files on Dropbox have the correct modified timestamp.
  2. Surprisingly, all the files have nearly the same mTimeRemote* values, which is of the "sync up". If it were the real modified timestamps, all files would have different mTimeRemote* values.
  3. Problem 2: More surprisingly, all files also have the same mTimeLocal*. I figured out that this is because the mTimeLocal* seems to be picking the "creation date" of the file in the local copy. And that's also from today since I just copied the vault to test the plugin, and MacOS seems to assign new "creation date" on file copy. The modification timestamp is preserved in the copy though, but the plugin seems to be picking the latest one from the "creation date" and the "modified date" when it should just pick modified date.

Thank you so much to both of you for your work on this so far!

@sboesen
Copy link
Owner

sboesen commented Jan 12, 2024

Really appreciate you posting the details, very odd behavior & sorry the previous changes didn't quite fix it for you. I plan to put the time I have into continuing trying to fix this in #97 which should provide a generic solution - but that does give me a hint for implementation. I think this line of code could be the culprit:

https://github.com/sboesen/remotely-sync/blob/master/src/sync.ts#L313

It's checking if the creation time is later than the modification time and if so using that - so maybe this is taking precedence over the locally copied modification time? Mostly speculation, no testing so far (sorry about that). I would be surprised (but certainly possible) that the sync from dropbox -> local file failed to preserve modification time, this does seem like it could be the issue.

Regardless will need to fix/update this condition but will do that as part of the generic solution unless @kadisonm has the time to fix this in the meantime.

@kadisonm
Copy link

I can certainly take a look. :)

@kadisonm
Copy link

kadisonm commented Jan 15, 2024

Is there any particular reason for the ctime later than mtime check? I assume this was done because otherwise the modification time is before the creation time.

MacOS seems to assign new "creation date" on file copy. The modification timestamp is preserved in the copy though

But having a look at how my windows machine handles copied files, I found the same thing as @Engineer-Extreme. So I don't think the check is necessary.

from dropbox -> local file failed to preserve modification time, this does seem like it could be the issue

After removing the check, I managed to make it upload the right modification time each "sync up". Oddly though, when I try to "sync down" on another vault, the modification time always has +5-10 seconds. Which I can confirm checking the file properties between the vaults, they don't match up.

I checked what the remotemtime is returning and it seems to be wrong. I believe the issue stems from here.

export const parseRemoteItems = async (

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Backlog
Development

No branches or pull requests

3 participants