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

Regression - 1.34 Insiders tries to open non-existent files in /.vscode folder #72891

Closed
gjsjohnmurray opened this issue Apr 26, 2019 · 9 comments
Assignees
Labels
config VS Code configuration, set up issues remote Remote system operations issues

Comments

@gjsjohnmurray
Copy link
Contributor

A FileSystemProvider extension that provides a /.vscode folder containing only a settings.json file also gets asked by 1.34.0-insider to readFile non-existent files named tasks.json and launch.json from that folder. This does not happen on 1.33.1

Steps to Reproduce:

  1. Obtain the MemFS sample (git clone https://github.com/Microsoft/vscode-extension-samples and then go to the fsprovider-sample subdir)
  2. In src/extension.ts add the following lines to the function that is registered as implementing the 'memfs.init' command (code pointer)
        // canary .vscode folder with stub settings.json file
        memFs.createDirectory(vscode.Uri.parse(`memfs:/.vscode/`));
        memFs.writeFile(vscode.Uri.parse(`memfs:/.vscode/settings.json`), Buffer.from('{\n}'), { create: true, overwrite: true });
  1. At the start of the stat method of the MemFS class in src/fileSystemProvider.ts (code pointer) add a logpoint with the following message: stat {uri.path}
  2. At the start of the readFile method (code pointer) add a logpoint with the following message: readFile {uri.path}
  3. Launch debugging using the 'Launch Extension' configuration.
  4. When the [Extension Development Host] window appears, switch to it and run (e.g. F1) the command MemFS: Setup Workspace followed by the command MemFS: Create Files

Debug Console output from 1.34.0-insider is:

MemFS says "Hello"
MemFS says "Hello"
readFile /.vscode/settings.json
readFile /.vscode/tasks.json
readFile /.vscode/launch.json
readFile /.vscode/settings.json
readFile /.vscode/tasks.json
readFile /.vscode/launch.json
stat /.vscode/settings.json
stat /.vscode/tasks.json
stat /.vscode/launch.json
stat /.vscode/settings.json
stat /.vscode/tasks.json
stat /.vscode/launch.json
stat /.vscode/tasks.json
stat /.vscode/launch.json
stat /.vscode/tasks.json
stat /.vscode/launch.json

Using 1.33.1 the same procedure gives:

MemFS says "Hello"
MemFS says "Hello"
stat /.vscode
stat /.vscode/settings.json
readFile /.vscode/settings.json

Notice how 1.33.1 stats the .vscode folder and its settings.json file before reading the file, whereas 1.34-insider reads that file (twice) and attempts to read the non-existent files tasks.json and launch.json (again, twice), only issuing stat calls on the files afterwards.

VS Code version: Code - Insiders 1.34.0-insider (0fde661, 2019-04-25T05:24:52.804Z)
OS version: Windows_NT x64 10.0.16299

@bpasero
Copy link
Member

bpasero commented Apr 26, 2019

@sandy081 possibly related to using the file service for configuration?

@bpasero bpasero added the config VS Code configuration, set up issues label Apr 26, 2019
@gjsjohnmurray
Copy link
Contributor Author

@bpasero - Maybe related to this, which looks to me like it could result in readFile getting ahead of stat, particularly if the FileSystemProvider doesn't supply an etag string.

@bpasero
Copy link
Member

bpasero commented Apr 26, 2019

@gjsjohnmurray that should be fine though as both stat and readFile will trigger the ENOTFOUND error, no? I think the question is if previously we used node APIs to do the reading without proper error handling and now through the file service errors are logged AND we go through the file providers.

@gjsjohnmurray
Copy link
Contributor Author

My FileSystemProvider is derived from RemoteFS, which notifies the user of errors during readFile but is quiet about ones in stat. I guess there may be other extensions out there which aren't going to be happy about this change.

Maybe related, when Insiders opens files served by my extension it appears to succeed (i.e. no errors) but the files display as though they are 0-byte ones. I can reproduce this with the RemoteFS extension connecting using FTP to a couple of different public sites. No such problems with the current 1.33.1 release. I'm about to open a new issue about this.

@bpasero
Copy link
Member

bpasero commented Apr 26, 2019

@gjsjohnmurray you do not need to notify the user about errors, this is taken care of by the file service in VSCode and in most cases even better because it can put the error in context (e.g. when you follow a link to a file that does not exist we can show an error with a button to create this file).

Do you have a sample I can try to reproduce the second issue you are talking about?

@bpasero
Copy link
Member

bpasero commented Apr 26, 2019

I can reproduce the many errors popping up on startup:

image

image

via the steps in #72909

This is not a very nice user experience, I have to agree.

@bpasero bpasero self-assigned this Apr 26, 2019
@bpasero bpasero added this to the April 2019 milestone Apr 26, 2019
@bpasero bpasero added bug Issue identified by VS Code Team member as probable bug remote Remote system operations issues labels Apr 26, 2019
@sandy081
Copy link
Member

sandy081 commented Apr 26, 2019

IMO, it is totally fine to read the files even if they do not exist.. extension do not need to log or notify such as the caller of the API (in this case me) should handle it.

@bpasero May I know who are logging/prompting these errors?

@bpasero
Copy link
Member

bpasero commented Apr 26, 2019

@sandy081 actually turns out this is coming from the extension itself and not from us:

https://github.com/liximomo/vscode-remote-fs/blob/88b67ff683a05998d79bd4fb8466f7ca518d0f51/src/helpers/reportError.ts#L6

@gjsjohnmurray
Copy link
Contributor Author

I'm closing this. I have revised my extension so it no longer outputs these messages. The author of Remote FS might do likewise in the future.

@bpasero bpasero removed the bug Issue identified by VS Code Team member as probable bug label Apr 27, 2019
@bpasero bpasero removed this from the April 2019 milestone Apr 27, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators Jun 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
config VS Code configuration, set up issues remote Remote system operations issues
Projects
None yet
Development

No branches or pull requests

3 participants