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

Attempt to open file as fallback for URIs without scheme #2527

Closed
wants to merge 1 commit into from

Conversation

ryuukk
Copy link
Contributor

@ryuukk ryuukk commented Oct 10, 2024

I kept getting error: Unable to open URI, i didn't know why

Turns out the plugin requires the path to have a scheme file://

I think it's fine to still try to open it as a fallback

@jwortmann
Copy link
Member

This function takes a DocumentUri as input parameter and per definition an URI must always have a scheme because the scheme part is mandatory.

When does this error happen? For example for links in a hover popup there is a fallback to open it in a browser if the link doesn't have a scheme:

LSP/plugin/hover.py

Lines 369 to 372 in d71e11a

elif parse_uri(href)[0].lower() in ("", "http", "https"):
open_in_browser(href)
else:
sublime.set_timeout_async(partial(self.try_open_custom_uri_async, href))

@ryuukk
Copy link
Contributor Author

ryuukk commented Oct 10, 2024

This function takes a DocumentUri as input parameter and per definition an URI must always have a scheme because the scheme part is mandatory.

When does this error happen? For example for links in a hover popup there is a fallback to open it in a browser if the link doesn't have a scheme:

LSP/plugin/hover.py

Lines 369 to 372 in d71e11a

elif parse_uri(href)[0].lower() in ("", "http", "https"):
open_in_browser(href)
else:
sublime.set_timeout_async(partial(self.try_open_custom_uri_async, href))

it happens when textDocument/definition (Goto Definition)

{'uri': '/run/media/ryuukk/E0C0C01FC0BFFA3C/dev/kdom/better_d/rt/time.d', 'range': {'start': {'line': 149, 'character': 16}, 'end': {'line': 149, 'character': 16}}}

i have fixed my server since then, but i still believe either a fallback or a proper message is better than returning None and giving a generic error message

@predragnikolic
Copy link
Member

predragnikolic commented Oct 10, 2024

I think it's fine to still try to open it as a fallback

The URI spec says the following:

The scheme and path components are required, though the path may be
empty

Ref: https://datatracker.ietf.org/doc/html/rfc3986#section-3.1

That said, this PR introduces a behavior that is not spec compliment complient.

a fallback or a proper message is better than returning None and giving a generic error message

When I saw the issue, I immediately knew that the error was due to a wrong document URI, I deduced that by the generic error message. So I'm fine withe the error message. if you have a suggestion on how the error message could be improved, do tell.

@ryuukk
Copy link
Contributor Author

ryuukk commented Oct 10, 2024

I think it's fine to still try to open it as a fallback

The URI spec says the following:

The scheme and path components are required, though the path may be
empty

Ref: https://datatracker.ietf.org/doc/html/rfc3986#section-3.1

That said, this PR introduces a behavior that is not spec compliment.

a fallback or a proper message is better than returning None and giving a generic error message

When I saw the issue, I immediately knew that the error was due to a wrong document URI, I deduced that by the generic error message. So I'm fine withe the error message. if you have a suggestion on how the error message could be improved, do tell.

Well, if URI is not empty, and scheme is required and missing, the error message should be:

Invalid URI, scheme is missing

I have wasted valuable time wondering what was wrong

@predragnikolic
Copy link
Member

True, the error message could can be improved. That would require a few regexes to be written. Not sure if the effort is worth the gain.

You could save time if you use AI, just to see different pints of views, if you are not sure what the error is.

ChatGPT spotted the issue, with the error message description that you provided.

ChatGPT:

The error message Unable to open URI when calling the textDocument/definition request suggests that there is an issue with resolving or accessing the file specified by the URI. Based on the provided URI, here are a few possible causes:

  1. Incorrect File Path:
    The URI /run/media/ryuukk/E0C0C01FC0BFFA3C/dev/kdom/better_d/rt/time.d might not be correct, or the file might not exist at that location. Double-check if the file exists and whether the LSP server has access to the location.

  2. File URI Scheme Mismatch:
    The URI uses an absolute file path without the file:// prefix (as per URI standards for file paths). Ensure the LSP server expects this format. The correct URI should look something like:

file:///run/media/ryuukk/E0C0C01FC0BFFA3C/dev/kdom/better_d/rt/time.d
  1. Permission Issues:
    If the file is on a mounted drive (/run/media/ryuukk/ suggests an external or USB drive), there could be permission issues preventing the LSP server from accessing the file. Ensure that the LSP server has the necessary read permissions on that file and the directory it's in.

  2. File System Accessibility:
    If the file is located on an external or network-mounted device, the LSP server might not have access to that device, or the device might be unmounted or not accessible during the request.

Steps to troubleshoot:

  • Check if the file exists at the given path and is accessible by your LSP server.
  • If the file is on a mounted device, ensure it’s properly mounted and readable.
  • Test with a file:// URI format to see if the LSP server accepts that format.
  • Inspect the LSP server logs for more specific error details that might point to the root cause.

By addressing these areas, you should be able to identify why the file cannot be opened.

@rchl
Copy link
Member

rchl commented Oct 14, 2024

Feel free to submit a PR that improves the error message.

But note that there is an infinite amount of non-compliant cases (not only for this case but in general) that the server could trigger and it's not very reasonable to expect that the client will handle every one of those in a way that provides detailed and very relevant response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants