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

FileSearchProvider and TextSearchProvider broken in 1.93.0 for folders with query parameters #227836

Open
isc-bsaviano opened this issue Sep 6, 2024 · 21 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug search Search widget and operation issues

Comments

@isc-bsaviano
Copy link

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
Copy link
Contributor

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

@isc-bsaviano
Copy link
Author

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:
Screenshot 2024-09-06 at 5 06 18 PM

@andreamah
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

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 #227248).

@isc-bsaviano
Copy link
Author

That works

@andreamah
Copy link
Contributor

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 andreamah added search Search widget and operation issues bug Issue identified by VS Code Team member as probable bug labels Sep 9, 2024
@andreamah andreamah added this to the September 2024 milestone Sep 9, 2024
@andreamah
Copy link
Contributor

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

@isc-bsaviano
Copy link
Author

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
Copy link
Contributor

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
Copy link
Author

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

@jrieken
Copy link
Member

jrieken commented Sep 9, 2024

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):

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

@andreamah
Copy link
Contributor

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
Copy link
Author

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

@andreamah
Copy link
Contributor

andreamah commented Sep 9, 2024

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
Copy link
Author

Yes, that's what I mean

@andreamah
Copy link
Contributor

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
Copy link
Member

jrieken commented Sep 10, 2024

Yeah, looks like missed the last update here

@andreamah
Copy link
Contributor

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
Copy link
Member

jrieken commented Sep 10, 2024

how/where do you use the TST for this?

@andreamah
Copy link
Contributor

I use it here

const folderMappings: TernarySearchTree<URI, FolderQueryInfo> = TernarySearchTree.forUris<FolderQueryInfo>();

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?

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 search Search widget and operation issues
Projects
None yet
Development

No branches or pull requests

4 participants