Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Display more specific JDT error messages #68

Closed
tylerFowler opened this issue Apr 3, 2018 · 9 comments
Closed

Display more specific JDT error messages #68

tylerFowler opened this issue Apr 3, 2018 · 9 comments

Comments

@tylerFowler
Copy link
Contributor

Just got an error on JDT startup with this message:

Java (Eclipse JDT) Error Could not run build action using Gradle distribution 'https://services.gradle.org/distributions/gradle-4.3.1-bin.zip'.

Which was displayed in the server status section of my statusbar, so it was truncated (had to use the inspector to view the HTML to actually get the whole message). Yet the warning I got as an Atom notification was the standard "Incomplete classpath" message. It would be nice to display these more specific errors in a notification, along with whatever possible information it can get from the JDT server as to why it wouldn't start.

@tylerFowler
Copy link
Contributor Author

Taking a bit of a closer look at this, it looks like atom-languageclient only listens in on the child's stderr stream. But the JDT server actually throws a bunch of useful info (e.g. stacktraces) into stdout using the method language/status and the error type:

{
  "jsonrpc": "2.0",
  "method": "language/status",
  "params": {
    "type": "Error",
    "message": "Could not run build action using Gradle distribution 'https://services.gradle.org/distributions/gradle-4.3.1-bin.zip'."
  }
}

Which is immediately preceded by (stacktrace truncated by me):

{
  "jsonrpc": "2.0",
  "method": "window/logMessage",
  "params": {
    "type": 1,
    "message": "Apr 3, 2018 5:00:30 PM Initialization failed \nCould not run build action using Gradle distribution https://services.gradle.org/distributions/gradle-4.3.1- bin.zip.\norg.gradle.tooling.BuildException: Could not run build action using Gradle distribution https://services.gradle.org/distributions/gradle-4.3.1-bin.zip.\n\tat  org.gradle.tooling.internal.consumer.ExceptionTransformer.transform(ExceptionTransformer.java:51)\n\tat org.gradle.tooling.internal.consumer.ExceptionTransformer.transform(ExceptionTransformer.java:29) ..."
  }
}

I can throw in a hook that displays notifications from these errors along with expandable stacktraces. @damieng Would it be better if this sort of thing just went into atom-languageclient following the pattern of captureServerErrors? Not sure how common it is for language servers to throw error messages onto stdout like this.

@damieng
Copy link
Contributor

damieng commented Apr 3, 2018

We do currently display logMessages in the new Console window in atom-ide-ui.

language/status is a non-standard message so we don't want to do anything in atom-languageclient with it there.

If you open the console window in atom-ide-ui (View > Toggle Console) does the error look good there? I wonder if we should just auto-show the Console if it receives an error?

@tylerFowler
Copy link
Contributor Author

No actually when I reproduce this specific JDT load error the console just stays blank. Which makes sense because I couldn't find anywhere in the code (in this repo or atom-languageclient) that actually hooks into the child process stdout stream. I actually wasn't really aware of the console.. I can take a closer look at it tonight.

@damieng
Copy link
Contributor

damieng commented Apr 4, 2018

You won't see anything looking specifically at stdin/stdout as that is what the vscode-jsonrpc library uses as its communication channel between the client and server when talking the language server protocol (it can also use sockets and ipc but for ide-java we just used stdin/stdout).

That message you see window/logMessage is a jsonrpc message that will come through atom-languageclient as a 'logMessage' notification and should be appearing in the devtools output window as well if you've turned on message logging with atom.config.set('core.debugLSP', true).

@tylerFowler
Copy link
Contributor Author

I see, yeah I do see the stacktraces when that config option is set. I get these "Incomplete classpath" errors enough that it would probably be worthwhile to add it to the console and possibly modify the warning notification with more specifics, at minimum a note telling the user to look at the console.

But if we get a language/status message like "Could not run build action using Gradle distribution" then I'd think the notification should say that instead, redirecting the user to the console for a stacktrace

@Arcanemagus
Copy link
Contributor

But if we get a language/status message like "Could not run build action using Gradle distribution" then I'd think the notification should say that instead, redirecting the user to the console for a stacktrace

language/status is a non-standard message so we don't want to do anything in atom-languageclient with it there.

You should file an issue on eclipse.jdt.ls about changing these non-standard messages to something that fits within the LSP spec 😉.

@damieng
Copy link
Contributor

damieng commented Apr 5, 2018

We currently map all those language/status messages to the status bar in ide-java - I think we might want to just make it bigger or put some kind of PR into status-bar to allow hover text for the full content.

@tylerFowler
Copy link
Contributor Author

I think a hover text would be ideal if it's kept in the status bar. I find I have just enough status bar based packages that a full message will rarely fit and I'm sure I'm not alone on that. At that rate though auto-focusing the Atom IDE console would also be a good idea (as mentioned before), from a UX perspective a truncated status bar message might be easy to miss.

@Arcanemagus Probably a good idea.. Definitely not advocating to handle anything non-standard in the language client package.

@tylerFowler
Copy link
Contributor Author

tylerFowler commented Apr 15, 2018

Looked into getting log messages into the Console, after adding console to consumedServices I can see that the atom-languageclient consumeConsole method is being called. But when it iterates over the active servers in the server manager to add LoggingConsoleAdapter's there aren't any active servers yet so the created console is never attached to the connection. So this should explain why window/logMessage types aren't being shown on the console.

Unsure what to do about this, but could be solved by adding a hook to the language client ServerManager to subscribe to new active servers or just overriding consumeConsole in this package and manually wiring it up.

Edit:
Er.. nevermind, I must've messed something up while I was playing around with console consumers. Simply adding console as a consumed service in the package.json file makes errors show up in the Console. Will submit a PR for that in a bit and that will probably close this issue altogether.

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

No branches or pull requests

3 participants