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

scala.util.Success.flatMap catching too many exceptions while it's documented to not do that #6190

Closed
scabug opened this issue Aug 5, 2012 · 2 comments
Assignees
Labels
Milestone

Comments

@scabug
Copy link

@scabug scabug commented Aug 5, 2012

While this is fine:

scala> val res = scala.util.Success(1).flatMap(x => throw new Exception)
res: scala.util.Try[Nothing] = Failure(java.lang.Exception)

The documentation seems to suggest that this is not expected (the exception should be thrown instead):

scala> val res = scala.util.Success(1).flatMap(x => throw new ThreadDeath)
res: scala.util.Try[Nothing] = Failure(java.lang.ThreadDeath)

Docs say that:

  • ''Note'': only non-fatal exceptions are caught by the combinators on Try (see [[scala.util.control.NonFatal]]).
  • Serious system errors, on the other hand, will be thrown.

Within class Success, the bug seems to be here (comment added):

  def flatMap[U](f: T => Try[U]): Try[U] =
    try f(value)
    catch {
      case e => Failure(e) //XXX This should be case NonFatal(e)!
    }
@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Aug 5, 2012

Imported From: https://issues.scala-lang.org/browse/SI-6190?orig=1
Reporter: @Blaisorblade
Affected Versions: 2.10.0-M6

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Aug 25, 2012

@Blaisorblade said (edited on Aug 25, 2012 5:00:30 PM UTC):
This was silently fixed in commit 3cb0e784a05db7d0b542cec9bf4c5fbf3772a6cf, at least judging from the source code. However, no test case was introduced for fatal exceptions — otherwise, I would close this issue.
UPDATE: I tried addressing this in scala/scala#1195.

@scabug scabug closed this Aug 29, 2012
@scabug scabug added the library label Apr 7, 2017
@scabug scabug added this to the 2.10.0-M7 milestone Apr 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.