`FileSearchProvider` and `TextSearchProvider` broken in 1.93.0 for folders with query parameters

[isc-bsaviano]

My extension's FileSearchProvider and TextSearchProvider report this error on version 1.93.0 for folders with query parameters in their URI:

TypeError: Cannot read properties of undefined (reading 'folder')

@andreamah Could this have been caused by your recent work in this area?

• VS Code Version: 1.93.0

[andreamah]

Hmm. Do the workspace folder queries have URIs? Or is it the URI that is returned for the matching file?

[isc-bsaviano]

The options object does have a folder URI:

{
  "folder": {
    "$mid": 1,
    "fsPath": "/",
    "external": "isfs://iris:user/?mapped%3D0",
    "path": "/",
    "scheme": "isfs",
    "authority": "iris:user",
    "query": "mapped=0"
  },
  "excludes": [
    "**/.git",
    "**/.svn",
    "**/.hg",
    "**/CVS",
    "**/.DS_Store",
    "**/Thumbs.db",
    "**/node_modules",
    "**/bower_components",
    "**/*.code-search"
  ],
  "includes": [],
  "useGlobalIgnoreFiles": false,
  "useIgnoreFiles": true,
  "useParentIgnoreFiles": false,
  "followSymlinks": true,
  "maxResults": 0,
  "session": {
    "a": false,
    "b": null
  }
}

However, I see this error in the Developer Tools output:

[andreamah]

Could you try with a workspace that doesn't have a query parameter in the workspace folder URI? I'm assuming that the query info has important info that you need for the search provider?

[isc-bsaviano]

The workspace I use has one folder with a query parameter, one without. I only saw one error in the Developer Tools. We use the query parameters to filter the files shown in VS Code since the virtual file systems can be large. Here's the file search options for the folder without query parameters:

{
  "folder": {
    "$mid": 1,
    "fsPath": "/",
    "external": "isfs://iris:%sys/",
    "path": "/",
    "scheme": "isfs",
    "authority": "iris:%sys"
  },
  "excludes": [
    "**/.git",
    "**/.svn",
    "**/.hg",
    "**/CVS",
    "**/.DS_Store",
    "**/Thumbs.db",
    "**/node_modules",
    "**/bower_components",
    "**/*.code-search"
  ],
  "includes": [],
  "useGlobalIgnoreFiles": false,
  "useIgnoreFiles": true,
  "useParentIgnoreFiles": false,
  "followSymlinks": true,
  "maxResults": 0,
  "session": {
    "a": false,
    "b": null
  }
}

[andreamah]

Could you try it out with just the workspace folder with no query parameter in its URI? I think I know what's happening (related to https://github.com/microsoft/vscode/issues/227248).

[isc-bsaviano]

That works

[andreamah]

We made a change to the API recently so that we do one call to the extension for results for all workspace folders. Then, we try to derive the original workspace folder by using the result's URI. I think that the latter is currently failing with query params - let me try to fix this.

[andreamah]

Question: do you ever have workspace folders that have the same URI except for the query parameter and/or the fragment?

[isc-bsaviano]

Yes, we do. The URI authority tells our extension what remote server to connect to, and the query parameters add filtering to limit the scope of documents on the remote server that are part of that VS Code workspace folder. There's no reason why a workspace couldn't contain two folders, both connected to the same remote server but with different filtering options. As of now we do not use the fragment, just query parameters.

[andreamah]

Hmm okay. @jrieken @mjbvz we probably need to re-think how we're deducing the original workspace folder of a result, since just finding which workspace folder these URIs live relative to seems to not be working for all API adopters.

[isc-bsaviano]

@andreamah FWIW we've experienced issues with this in the past. See #212363 for a recent, still-open example.

[jrieken]

Hmm okay. @jrieken @mjbvz we probably need to re-think how we're deducing the original workspace folder of a result, since just finding which workspace folder these URIs live relative to seems to not be working for all API adopters.

I believe you need to make sure to honor the query-part which isn't the default. I don't really know if or how you do this but the search tree does have support for it (ignoreQueryAndFragment argument): https://github.com/microsoft/vscode/blob/1349397cf767bda1680b22c095cf18084e71cf43/src/vs/base/common/ternarySearchTree.ts#L306

[andreamah]

Does the URI need to have the query/fragment, though? For example, if we have the workspace isfs://iris:user/?mapped%3D0, does the URI need to look like isfs://iris:user/foo/bar?mapped%3D0?

@isc-bsaviano are you returning the file result URIs with the query attached?

[isc-bsaviano]

Yes, the returned file URIs contain the query parameters. They only differ from the containing workspace folder URI in the path segment.

[andreamah]

They only differ from the containing workspace folder URI in the path segment.

Could you give an example of this? Do you mean that the only difference is the path that they have, since the result is in a subfolder of the workspace folder?

[isc-bsaviano]

Yes, that's what I mean

[andreamah]

Hmm okay. @jrieken @mjbvz we probably need to re-think how we're deducing the original workspace folder of a result, since just finding which workspace folder these URIs live relative to seems to not be working for all API adopters.

I believe you need to make sure to honor the query-part which isn't the default. I don't really know if or how you do this but the search tree does have support for it (ignoreQueryAndFragment argument):

vscode/src/vs/base/common/ternarySearchTree.ts

Line 306 in 1349397

static forUris(ignorePathCasing: (key: URI) => boolean = () => false, ignoreQueryAndFragment: (key: URI) => boolean = () => false): TernarySearchTree<URI, E> {

if the default is defined using ignoreQueryAndFragment: (key: URI) => boolean = () => false, doesn't that mean that this doesn't ignore by default? Seems that it's a function that resolves to false by default. @jrieken

[jrieken]

Yeah, looks like missed the last update here

[andreamah]

If the issue isn't with the TernarySearchTree, I'll need to try to replicate the scenario to see where the error is. My original assumption was also that it was a TernarySearchTree issue.

[jrieken]

how/where do you use the TST for this?

[andreamah]

I use it here https://github.com/microsoft/vscode/blob/f4a34d0d7a7d1702e36b8e9a929b680c6ed5c6c8/src/vs/workbench/services/search/common/fileSearchManager.ts#L128

I use findSubstr to try to find whether a URI is within a parent folder. Do you know what could be causing the error?

@isc-bsaviano do you happen to have a small repro project that I can try out to run into the bug?

[isc-bsaviano]

@andreamah Sorry, I was away for a while. Do you still need help reproducing this?

[andreamah]

Yes, do you mind putting together a small project that reproduces the issue?

[isc-bsaviano]

@andreamah Attached is a modified version of the fsprovider-sample extension that demonstrates the issue. Here are the steps to reproduce:

1. Enable proposed API for extension vscode-samples1.vscode-memfs.
2. Download the zip, unzip it, and install the extension VSIX.
3. When not having a workspace opened, run the MemFS Setup Workspace command.
4. Run the MemFS Create Files command.
5. Run the Toggle Developer Tools command.
6. Type Ctrl-P to trigger the FileSearchProvider.
7. Observe the error message written to the dev tools console.

vscode-memfs-0.0.3.vsix.zip

[andreamah]

@jrieken When the TST sets ignoreQueryAndFragment to false, does findSubstr still find roots with the same query parameter?

For example, running this test:

    test('TernarySearchTree (URI) - query parameters', function () {
        const trie = new TernarySearchTree<URI, number>(new UriIterator(() => false, () => false));
        const root = URI.parse('memfs:/?q=1');
        trie.set(root, 1);

        assert.strictEqual(trie.get(URI.parse('memfs:/?q=1')), 1);

        assert.strictEqual(trie.findSubstr(URI.parse('memfs:/file.txt?q=1')), 1);
    });

Should I expect it to pass?

[jrieken]

Yes, this is expected to pass because they query part q=1 is equal in both uris

[gjsjohnmurray]

Linking to #212436 in case relevant. Even if not it'd be good to get that PR merged as it will fix another issue affecting @isc-bsaviano

[andreamah]

Yes, this is expected to pass because they query part q=1 is equal in both uris

@jrieken it does not pass. I tried it in the TST tests.

[gjsjohnmurray]

@andreamah please see #230483 for why the urgency of getting this fixed has increased. It now affects Explorer Find on folders that use our extension's FileSystemProvider for which queryparams are often essential to its uris working.

[andreamah]

That seems not related to having query parameters (?) According to the repro steps, it should happen regardless of having a query parameter?

[isc-bsaviano]

Hi @andreamah, any updates on this? This issue is forcing a bunch of our end users to be stuck on 1.92.2.

[andreamah]

@jrieken do you know why the test case in https://github.com/microsoft/vscode/issues/227836#issuecomment-2387244287 fails?