Metals: sbt server integration has regression

[olafurpg]

While trying out the sbt server integration again I hit on errors I have not seen before

$ cat .metals/metals.log
00:13:58.021 ERROR s.m.m.s.SbtServer$ - Response to unknown request: Success({
  "status" : "Done",
  "channelName" : "network-2",
  "execId" : "2",
  "commandQueue" : [
    "shell"
  ],
  "exitCode" : 0
},String("2"))
00:13:58.021 ERROR s.m.m.s.SbtServer$ - Response to unknown request: Success({
  "status" : "Done",
  "channelName" : "network-2",
  "execId" : "2",
  "commandQueue" : [
    "shell"
  ],
  "exitCode" : 0
},String("2"))
00:13:58.383 ERROR s.m.m.s.SbtServer$ - Response error: Error(ErrorObject(InvalidRequest,DecodingFailure(DecodingFailure(-33000 is not a member of enum ErrorCode, List(DownField(code), DownField(error))), List()),None),Null)
$ sbt logs
> Exception in thread "sbt-networkchannel-0" sjsonnew.DeserializationException: Expected Long as JNumber, but got JNull

We should investigate this and check why this is not caught by the full integration test.

[laughedelic]

This is related to the recent refactoring in the sbt server 1.1.2. It works well with 1.1.1. Somewhere here: https://github.com/sbt/sbt/pull/3989 and https://github.com/sbt/sbt/pull/3991.

I see 2 problems so far:

• it sends responses instead of notifications for the Done events
• it sends -33000 error code which doesn't exist in the LSP specification

Both things have to be fixed in sbt. I will report them.

[olafurpg]

Thanks for investigating @laughedelic ! Seems like best workaround is to use sbt 1.1.1 for now.

[eed3si9n]

According to JSON-PRC, sbt server should be allowed to send any error code outside of the reserved range. http://www.jsonrpc.org/specification#error_object

The remainder of the space is available for application defined errors.

[ShaneDelmore]

LSP is not fully JSON-RPC compliant. It’s unfortunate, but finding an actually JSON-RPC compliant api is as rare as finding a REST API that Roy Fielding agrees is actual REST.

[dwijnand]

it sends responses instead of notifications for the Done events

why are you saying that that is a problem?

[laughedelic]

@dwijnand sorry, this was a wrong statement. I should have updated it after I looked into the changes in https://github.com/sbt/sbt/pull/3991 in more details. I made this conclusion because I thought it's similar to the old situation in https://github.com/sbt/sbt/pull/3712.

So currently sbt does reply correctly to the sbt/exec requests with the Done ExecStatusEvent response 👍 The problem now is that after connecting to the sbt server, it emits one of these Done responses without any sbt/exec request:

13:16:41.785 ERROR s.m.m.s.SbtServer$ - Response to unknown request: Success({
  "status" : "Done",
  "channelName" : "network-3",
  "execId" : "2",
  "commandQueue" : [
    "shell"
  ],
  "exitCode" : 0
},String("2"))

(probably it's a response to the initialization request?)

Also if I reload sbt shell while there is an open connection, the client receives some _responses_ that weren't meant for it (wrong channel):

13:19:18.194 ERROR s.m.m.s.SbtServer$ - Response to unknown request: Success({
  "status" : "Done",
  "channelName" : "console0",
  "execId" : "6f7b96a9-5eed-4098-acc9-2fd3a654eb4c",
  "commandQueue" : [
    "shell"
  ],
  "exitCode" : 0
},String("6f7b96a9-5eed-4098-acc9-2fd3a654eb4c"))
13:19:18
13:19:20.487 ERROR s.m.m.s.SbtServer$ - Response to unknown request: Success({
  "status" : "Done",
  "channelName" : "console0",
  "execId" : "aabe6bf4-f24c-4804-97c3-0802570192a8",
  "commandQueue" : [
    "sbtStashOnFailure",
    "onFailure load-failed",
    "loadp",
    "sbtPopOnFailure",
    "resumeFromFailure",
    "shell"
  ],
  "exitCode" : 0
},String("aabe6bf4-f24c-4804-97c3-0802570192a8"))

[dwijnand]

(probably it's a response to the initialization request?)

kind of, it's in response to https://github.com/sbt/sbt/blob/v1.1.3/main/src/main/scala/sbt/internal/server/LanguageServerProtocol.scala#L67. which is unfortunate (to put politely....)

Also if I reload sbt shell while there is an open connection, the client receives some responses that weren't meant for it (wrong channel):

I apologise - it's possible my change inadvertently caused those regressions. if you have a chance to reproduce and report them I'll try and look into and fix them.

[laughedelic]

kind of, it's in response to sbt/sbt:main/src/main/scala/sbt/internal/server/[email protected]#L67. which is unfortunate (to put politely....)

Could this be fixed by simply passing None in place of the request ID?

if you have a chance to reproduce and report them I'll try and look into and fix them.

Sure. I didn't look into this recently, is there some testing infrastructure for the sbt server? Or you mean just submitting a bug report?

[dwijnand]

Could this be fixed by simply passing None in place of the request ID?

oh.. yeah, probably!

Sure. I didn't look into this recently, is there some testing infrastructure for the sbt server? Or you mean just submitting a bug report?

yeah there's https://github.com/sbt/sbt/blob/1.x/sbt/src/test/scala/sbt/ServerSpec.scala now. but I just meant a bug report (I like to make sure my mistakes are fixed).

[laughedelic]

@dwijnand done: https://github.com/sbt/sbt/issues/4091

[olafurpg]

Superseded by https://github.com/sbt/sbt/issues/4091 which seems to have a merged fix

[laughedelic]

@olafurpg https://github.com/sbt/sbt/issues/4091 is open because it's still not fixed (details). I also don't know what is the current situation with respect to lsp4s and the custom error message from sbt. In any case, I'm OK with closing this issue and reopening more specific ones later.