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

Failed to save 'index.tsx': The content of the file is newer #5943

Closed
hediet opened this issue Apr 23, 2024 · 15 comments · Fixed by #5958 or #5981
Closed

Failed to save 'index.tsx': The content of the file is newer #5943

hediet opened this issue Apr 23, 2024 · 15 comments · Fixed by #5958 or #5981
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug verification-found Issue verification failed
Milestone

Comments

@hediet
Copy link
Member

hediet commented Apr 23, 2024

Testing #5939

Failed to save 'index.tsx': The content of the file is newer. Please compare your version with the file contents or overwrite the content of the file with your changes.

chrome_f0ohKlDYwq

@alexr00 alexr00 added the bug Issue identified by VS Code Team member as probable bug label Apr 23, 2024
@alexr00 alexr00 added this to the April 2024 milestone Apr 23, 2024
@benibenj
Copy link
Contributor

benibenj commented Apr 23, 2024

I also saw the same problem on desktop. Did not get the notification but the resulting file consisted of random content unrelated to the file.
image

@alexr00
Copy link
Member

alexr00 commented Apr 24, 2024

@bpasero maybe you can help with a better solution. What happens:

  1. User enters conflict resolution mode. This causes GHPR to register a file system provider for the merge output.
  2. When the merge editor opens, it reads the merge output file.
  3. When the user exits conflict resolution mode, the file system provider is disposed.
  4. User enters conflict resolution mode again. The file system provider for the merge output is re-registered.
  5. When the merge editor opens, it reads the merge output file, but instead of reading it from the files system provider, it reads whatever the text model had from last time, even though a new file system provider was registered.

What I would like is when the file system provider is disposed, VS Code deletes all text models that used to "belong" to the file system provider. I haven't found a way to do this though. My quick fix is to use a different scheme each time I register the file system provider.

@bpasero
Copy link
Member

bpasero commented Apr 24, 2024

What I would like is when the file system provider is disposed, VS Code deletes all text models that used to "belong" to the file system provider

I think that is dangerous and could result in data loss. I might have an extension installed that allows to make changes to an editor with a custom FSP and scheme and when I disable that extension, my changes are lost? We are very very restrictive when to dispose a text file model, we actually only allow it unless the changes have been discarded or saved.

@bpasero
Copy link
Member

bpasero commented Apr 24, 2024

Maybe I do not fully understand why the file system provider is created and disposed when this happens. We do have a similar flow in core where we have a custom FSP only for resolving dirty writes (the conflict resolution to this error).

@alexr00
Copy link
Member

alexr00 commented Apr 24, 2024

@bpasero what if I fire a Deleted file change event from my file system provider before disposing? Is there any way a file system provider can tell VS Code that next time a file for that provider is opened it should go to the file system provider for the data?

@alexr00
Copy link
Member

alexr00 commented Apr 24, 2024

I only need the file system provider to be registered when in conflict resolution mode, that's why I create and dispose it when entering and leaving conflict resolution mode.

@bpasero
Copy link
Member

bpasero commented Apr 24, 2024

When the merge editor opens, it reads the merge output file, but instead of reading it from the files system provider, it reads whatever the text model had from last time, even though a new file system provider was registered.

There may be a bug here that we can fix, but to clarify: if an editor is dirty (has changes), we never go to the file system to resolve it but always resolve it from the editor model because the file will not have the changes and the content is in memory anyway. To remove these changes you would have to trigger the command to revert the editor.

Otherwise, if not dirty: In order to prevent going to the file system if there are no changes, we actually have a logic in place to figure out if the file has changed on disk compared to the version we have in memory (this requires the text model to be still around and not disposed). This depends on mtime and size information from the stat method:

https://github.com/microsoft/vscode/blob/0ed0a9fe1395f8263ba31f7a20cf8987666ac21d/src/vs/platform/files/common/fileService.ts#L701-L704

Maybe thats whats happening here? Are you implementing mtime and size properly in your FSP?

See also the docs around this:

https://github.com/microsoft/vscode/blob/0ed0a9fe1395f8263ba31f7a20cf8987666ac21d/src/vscode-dts/vscode.d.ts#L8732-L8747

@alexr00
Copy link
Member

alexr00 commented Apr 24, 2024

if an editor is dirty (has changes)

Autosave was on. I also closed the editor and wasn't prompted about dirty files.

This depends on mtime and size information from the stat method. Maybe thats whats happening here? Are you implementing mtime and size properly in your FSP?

Pretty sure I have this correct. stat is not even called before the editor is shown. The editor is first shown with the old model content, then stat is called, then it updates to the content that it should have started with.

@bpasero
Copy link
Member

bpasero commented Apr 24, 2024

Yeah thats true, another optimisation is that opening a model that is in-memory (e.g. still opened as tab) does not wait for the file service call but would show as it is in-memory and then try to see if newer content is there. Otherwise opening an existing tab would block for the duration of disk IO.

@alexr00
Copy link
Member

alexr00 commented Apr 25, 2024

This optimization makes sense, but I think as an extension I expect that if the tab is closed and I have disposed my filesystem provider then the in-memory model would not be used.

I've worked around this by having my file system provider register with a different scheme each time.

@bpasero
Copy link
Member

bpasero commented Apr 25, 2024

@alexr00 yeah so another thing that likely hits you is that text models are kept in-memory even after closing the tab for I think 3 minutes. @jrieken would be able to tell more, I think we do this because there is no explicit lifecycle in extension API to dispose a text model that has been opened (but I maybe wrong).

Refs: https://github.com/microsoft/vscode/blob/84d2ef91c53174337753c0970365dfa64b34b1b7/src/vs/workbench/api/browser/mainThreadDocuments.ts#L31-L38

@bhavyaus bhavyaus added the verification-steps-needed Steps to verify are needed for verification label Apr 25, 2024
@andreamah
Copy link

@alexr00 @bpasero are there verification steps for this?
@hediet is this still happening for you?

@alexr00
Copy link
Member

alexr00 commented Apr 26, 2024

To verify:

  1. Make sure you have the latest pre-release version installed
  2. Set "githubPullRequests.experimentalUpdateBranchWithGitHub": true
  3. Have a PR with a merge conflict do not check it out.
  4. Open the PR description and scroll down to the checks section to click on the "resolve conflicts" button
  5. Don't make any changes to the conflicting files, just "Accept merge" and ack the dialogs
  6. In the conflict resolution view, "Exit without merging"
  7. Repeat step 4.
  8. Verify that you see the correct content in the merge output and not some conflict markers.

@alexr00 alexr00 removed the verification-steps-needed Steps to verify are needed for verification label Apr 26, 2024
@andreamah andreamah added the verified Verification succeeded label Apr 26, 2024
@andreamah
Copy link

hmm, I get an error
image

I tried with this repo https://github.com/andreamah/test-repo and this branch andreamah/test-repo#1

@andreamah andreamah reopened this Apr 26, 2024
@andreamah andreamah added verification-found Issue verification failed and removed verified Verification succeeded labels Apr 26, 2024
@lramos15 lramos15 modified the milestones: April 2024, May 2024 Apr 26, 2024
@alexr00 alexr00 removed this from the May 2024 milestone Apr 29, 2024
@alexr00 alexr00 added this to the April 2024 milestone Apr 29, 2024
@alexr00 alexr00 added candidate Issue identified as probable candidate for fixing in the next release and removed candidate Issue identified as probable candidate for fixing in the next release labels Apr 29, 2024
@alexr00 alexr00 modified the milestones: April 2024, May 2024 Apr 29, 2024
@alexr00
Copy link
Member

alexr00 commented Apr 30, 2024

Good find @andreamah!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue identified by VS Code Team member as probable bug verification-found Issue verification failed
Projects
None yet
7 participants