Skip to content

[BSP] Send diagnostics and meaningful error message when reloading fails #6566

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Jul 12, 2021
Merged

[BSP] Send diagnostics and meaningful error message when reloading fails #6566

merged 5 commits into from
Jul 12, 2021

Conversation

adpi2
Copy link
Member

@adpi2 adpi2 commented Jun 25, 2021 •
edited
Loading

Fix #6010 and address scalameta/metals-feature-requests#143 (comment) (@ckipp01)

It should be probably merged after #6553 to avoid conflicts.

Just as #6553, merging this PR depends on whether we decide to break the binary compatibility of PluginData or not. In which case it would probably goes to sbt 1.6.

In order of commits:

  1. Simplify the bspReload command to catch the exceptions and fill the error response with the correct message
  2. Add a BuildServerEvalReporter to send diagnostics when evaluating .sbt files
  3. Add a test on a failing workspace/reload request

In action within Metals, it looks like this:

metals-reload-1

@adpi2 adpi2 mentioned this pull request Jun 28, 2021
Merged
@adpi2 adpi2 changed the title [BSP] Send meaningful error message when reloading fails [BSP] Send diagnostics and an meaningful error message when reloading fails Jun 28, 2021
@adpi2 adpi2 changed the title [BSP] Send diagnostics and an meaningful error message when reloading fails [BSP] Send diagnostics and meaningful error message when reloading fails Jun 28, 2021
@adpi2 adpi2 marked this pull request as ready for review June 28, 2021 16:13
@ckipp01
Copy link
Contributor

ckipp01 commented Jul 1, 2021

@adpi2 I feel like locally I'm doing something wrong when testing this out with Metals.

  1. I checkout out this pr
  2. Did a sbt publishLocal
  3. Switched my build to use the snapshot I just made
  4. Generated by bspConfig
  5. Then attached via bsp-switch

I can't seem to get the diagnostics to publish. I do see that it attaches to the new snapshot (I even bumped the version up in sbt to ensure I was really getting a fresh version).

BSP logs 
[Trace - 09:51:42 AM] Sending request 'workspace/reload - (14)'
Params: null


[Trace - 09:51:42 AM] Received notification 'build/logMessage'
Params: {
  "type": 4,
  "message": "Processing workspace/reload"
}


[Trace - 09:51:42 AM] Received notification 'build/taskStart'
Params: {
  "taskId": {
    "id": "50",
    "parents": []
  },
  "eventTime": 1625125902599,
  "message": "Compiling global-plugins",
  "dataKind": "compile-task",
  "data": {
    "target": {
      "uri": "file:/Users/ckipp/.sbt/1.0/plugins/#global-plugins/Compile"
    }
  }
}


[Trace - 09:51:42 AM] Received notification 'build/taskFinish'
Params: {
  "taskId": {
    "id": "50",
    "parents": []
  },
  "eventTime": 1625125902605,
  "message": "Compiled global-plugins",
  "status": 1,
  "dataKind": "compile-report",
  "data": {
    "target": {
      "uri": "file:/Users/ckipp/.sbt/1.0/plugins/#global-plugins/Compile"
    },
    "errors": 0,
    "warnings": 0,
    "time": 6
  }
}


[Trace - 09:51:42 AM] Received notification 'build/taskStart'
Params: {
  "taskId": {
    "id": "51",
    "parents": []
  },
  "eventTime": 1625125902967,
  "message": "Compiling sanity-build-build",
  "dataKind": "compile-task",
  "data": {
    "target": {
      "uri": "file:/Users/ckipp/Documents/scala-workspace/sanity/project/project/#sanity-build-build/Compile"
    }
  }
}


[Trace - 09:51:42 AM] Received notification 'build/taskFinish'
Params: {
  "taskId": {
    "id": "51",
    "parents": []
  },
  "eventTime": 1625125902975,
  "message": "Compiled sanity-build-build",
  "status": 1,
  "dataKind": "compile-report",
  "data": {
    "target": {
      "uri": "file:/Users/ckipp/Documents/scala-workspace/sanity/project/project/#sanity-build-build/Compile"
    },
    "errors": 0,
    "warnings": 0,
    "time": 8
  }
}


[Trace - 09:51:43 AM] Received notification 'build/taskStart'
Params: {
  "taskId": {
    "id": "52",
    "parents": []
  },
  "eventTime": 1625125903650,
  "message": "Compiling sanity-build",
  "dataKind": "compile-task",
  "data": {
    "target": {
      "uri": "file:/Users/ckipp/Documents/scala-workspace/sanity/project/#sanity-build/Compile"
    }
  }
}


[Trace - 09:51:43 AM] Received notification 'build/taskFinish'
Params: {
  "taskId": {
    "id": "52",
    "parents": []
  },
  "eventTime": 1625125903659,
  "message": "Compiled sanity-build",
  "status": 1,
  "dataKind": "compile-report",
  "data": {
    "target": {
      "uri": "file:/Users/ckipp/Documents/scala-workspace/sanity/project/#sanity-build/Compile"
    },
    "errors": 0,
    "warnings": 0,
    "time": 9
  }
}


[Trace - 09:51:44 AM] Received response 'workspace/reload - (14)' in 2143ms
Result: null
Error: {
  "code": -32603,
  "message": "reload failed"
}

I do get a whole bunch of

2021.07.01 09:51:41 ERROR java.lang.UnsupportedOperationException: empty.max
        at scala.collection.TraversableOnce.max(TraversableOnce.scala:275)
        at scala.collection.TraversableOnce.max$(TraversableOnce.scala:273)
        at scala.collection.AbstractTraversable.max(Traversable.scala:108)
        at sbt.internal.server.BuildServerProtocol$.$anonfun$globalSettings$40(BuildServerProtocol.scala:164)
        at sbt.internal.server.BuildServerProtocol$.$anonfun$globalSettings$40$adapted(BuildServerProtocol.scala:163)
        at scala.Function1.$anonfun$compose$1(Function1.scala:49)
        at sbt.internal.util.$tilde$greater.$anonfun$$u2219$1(TypeFunctions.scala:62)
        at sbt.std.Transform$$anon$4.work(Transform.scala:68)
        at sbt.Execute.$anonfun$submit$2(Execute.scala:282)
        at sbt.internal.util.ErrorHandling$.wideConvert(ErrorHandling.scala:23)
        at sbt.Execute.work(Execute.scala:291)
        at sbt.Execute.$anonfun$submit$1(Execute.scala:282)
        at sbt.ConcurrentRestrictions$$anon$4.$anonfun$submitValid$1(ConcurrentRestrictions.scala:265)
        at sbt.CompletionService$$anon$2.call(CompletionService.scala:64)
        at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
        at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
        at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
        at java.base/java.lang.Thread.run(Thread.java:829)

So I thought maybe that was causing some issues, but even removing the max call in bspBuildTargetCompile still was giving me this, which makes me feel like I'm not correctly using the snapshot I just published. Am I doing something wrong or missing a step to use this snapshot?

@adpi2
Copy link
Member Author

adpi2 commented Jul 1, 2021

From the logs it seems you don't have the snapshot running. Because the following message does not exist anymore (in this PR):

Error: {
  "code": -32603,
  "message": "reload failed"
}

When you change the sbt version in the build.properties file, you can try this:

  • rm -Rf ~/.sbt/boot/scala-2.12.14/org.scala-sbt/sbt/1.5.5-SNAPSHOT to remove the cached jars
  • run the sbt reboot command (reload is not enough) or kill the sbt process and start it again

If it is still not working you can try to clean the project/target directory. But there may be something wrong in this PR as well...

@ckipp01
Copy link
Contributor

ckipp01 commented Jul 1, 2021

  • rm -Rf ~/.sbt/boot/scala-2.12.14/org.scala-sbt/sbt/1.5.5-SNAPSHOT to remove the cached jars

Ahh bingo, that's what I was missing. Thanks! Back in business. This is fantastic that this is working.

Screenshot 2021-07-01 at 12 17 21

@adpi2
Simplify bspReload cmd by calling doLoadProject
4804cc2 
Fixes sbt#6010: Send real error message when realod failed
@adpi2 adpi2 force-pushed the fix-6010 branch 2 times, most recently from 721d955 to a923905 Compare July 8, 2021 07:14
adpi2 added 3 commits July 8, 2021 09:25
@adpi2
[BSP] send diagnostics when evaluating build.sbt
a76e209 
Since build.sbt is compiled/evaluated in `sbt.compiler.Eval`,
this commit introduces a `BuildServerEvalReporter` to redirect
the compiler errors to the BSP clients.

A new `finalReport` method is added in the new `EvalReporter` base class
to reset the old diagnostics.
@adpi2
[BSP] Add test on failing workspace/reload
b4c6299
@adpi2
Fix mima binary issues
94cc9a8
@adpi2 adpi2 requested a review from eed3si9n July 8, 2021 08:07
eed3si9n
eed3si9n previously approved these changes Jul 9, 2021
View reviewed changes
Copy link
Member

@eed3si9n eed3si9n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comment about Scaladoc, but other than that LGTM

@eed3si9n
Copy link
Member

eed3si9n commented Jul 9, 2021

Could you run scalafmt please?

@eed3si9n eed3si9n merged commit d4162cc into sbt:develop Jul 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eed3si9n eed3si9n eed3si9n approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BSP] Add details in the workspace/reload error response
3 participants
@adpi2 @ckipp01 @eed3si9n