Metals: Non Terminating Worksheet Evaluation for SO Condition

[robbyki]

Describe the bug
Code that returns an SO in a normal App causes a non-terminating Evaluated Worksheet message and does not return an SO.

To Reproduce
Below code should reproduce issue within a worksheet.sc file.

def bigData = (1 to 1000000).toStream
def foldLeft[A, B](l: List[A], z: B)(f: (B, A) => B): B = l match {
  case Nil    => z
  case h :: t => foldLeft(t, f(z, h))(f)
}
def foldRightViaFoldLeft[A, B](l: List[A], z: B)(s: (A, B) => B): B =
  foldLeft(l, (x: B) => x)((e, f) => (x: B) => e(s(f, x)))(z)
foldRightViaFoldLeft(bigData.toList, 0)(_ + _) // doesn't terminate

Installation:

• Operating system: Linux
• Editor: Visual Studio Code
• Metals version:

metals.serverVersion": "0.8.0+294-4dcdec07-SNAPSHOT

[tgodzik]

Thanks for the reproduction! That is for sure a bug!

[olafurpg]

Thank you for reporting! I believe this is caused by the fact that mdoc only catches non-fatal exceptions during evaluation. This is probably one of those places where we should catch e: Throwable 🤔

[kpbochenek]

Error thrown in this example is:

Caused by: java.lang.StackOverflowError
    at scala.collection.immutable.Nil$.equals(List.scala:435)
    at repl.Session$App.foldLeft(test.worksheet.sc:21)
    at repl.Session$App.foldLeft(test.worksheet.sc:22)
    at repl.Session$App.foldLeft(test.worksheet.sc:22)
    at repl.Session$App.foldLeft(test.worksheet.sc:22)
...

It is not caught here:

          try result.complete(Some(evaluateWorksheet(path, token)))
          catch {
            case e @ (NonFatal(_) | InterruptException()) =>
              result.completeExceptionally(e)
          }

Adding catching e: java.lang.StackOverflowError => ... solves a problem of infinite evaluation.
It might be changed to something more generic if needed of course.

But I can see here 2 problems:
1) With this change only there is no visual feedback to the user (in VS Code) that evaluation failed.
2) When evaluation fails it should clear previous evaluation results because it might be super misleading. E.g. when a code in worksheet is changed and no longer compiles previous evaluations are erased. I think it would make sense for similar thing in case of failed evaluation.

[tgodzik]

I agree that we should most likely catch that in Mdoc, since it might happen in other situations.

[olafurpg]

Closed #1495 and #1462 in favor of this ticket. It looks like a lot of users are hitting on this issue.

[olafurpg]

Opened https://github.com/scalameta/mdoc/pull/304, which should fix this issue once we upgrade to the next version of mdoc. Will follow up with a Metals PR

[olafurpg]

Open PR fixing this issue (along with duplicated issues) https://github.com/scalameta/metals/pull/1498