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
Failure.get should wrap exception before throwing #8963
Comments
Imported From: https://issues.scala-lang.org/browse/SI-8963?orig=1 |
@jrudolph said: My reasoning why this change is still a net-improvement is that you can always match on an expression of type Calling |
@jrudolph said: Caused by: akka.pattern.AskTimeoutException: Ask timed out on [Actor[akka://default/user/handler]] after [500 ms]
at akka.pattern.PromiseActorRef$$anonfun$1.apply$mcV$sp(AskSupport.scala:333)
at akka.actor.Scheduler$$anon$7.run(Scheduler.scala:117)
at scala.concurrent.Future$InternalCallbackExecutor$.unbatchedExecute(Future.scala:599)
at scala.concurrent.BatchingExecutor$class.execute(BatchingExecutor.scala:109)
at scala.concurrent.Future$InternalCallbackExecutor$.execute(Future.scala:597)
at akka.actor.LightArrayRevolverScheduler$TaskHolder.executeTask(Scheduler.scala:467)
at akka.actor.LightArrayRevolverScheduler$$anon$8.executeBucket$1(Scheduler.scala:419)
at akka.actor.LightArrayRevolverScheduler$$anon$8.nextTick(Scheduler.scala:423)
at akka.actor.LightArrayRevolverScheduler$$anon$8.run(Scheduler.scala:375)
at java.lang.Thread.run(Thread.java:745) My code doesn't use the ask pattern and my code doesn't use Await (but of course, any third party code I depend on may, I don't know). It only happens on production where I cannot attach a debugger. If |
@retronym said (edited on Nov 6, 2014 1:15:16 PM UTC): def prop(t: Throwable): Boolean = try { Try(throw t).get } catch t1: Throwable => t1 eq t } However we might consider using |
@jrudolph said: def prop(t: Throwable): Boolean = try { Try(throw t) match { case Success(t) => t } } catch t1: Throwable => t1 eq t } |
@jrudolph said: |
@retronym said: Java's use of wrapper exceptions may also be motivated by Java's checked exceptions. |
@jrudolph said: But stepping back a bit, maybe let's focus on the problem. My basic problem is that val x: Option[Int]
x.get
// =
// assert(x.isDefined, "programmer assumed that x.isDefined")
// x.asInstanceOf[Some].value Here's now an example showing the inconsistency: def optionTest(x: Option[Int]): Unit = {
x.get // oops, unguarded access, bug in this line
// Exception points to the buggy line:
// java.util.NoSuchElementException: None.get
// at scala.None$.get(Option.scala:313)
// at scala.None$.get(Option.scala:311)
// at example.GetInconsistence$.optionTest(GetInconsistence.scala:6)
}
def eitherTest(x: Either[String, Int]): Unit = {
x.right.get // oops, unguarded access, bug in this line
// Exception points to the buggy line:
// java.util.NoSuchElementException: Either.right.value on Left
// at scala.util.Either$RightProjection.get(Either.scala:454)
// at example.GetInconsistence$.eitherTest(GetInconsistence.scala:17)
}
def tryTest(x: Try[Int]): Unit = {
x.get // oops, unguarded access, bug in this line
// Exception doesn't include buggy line at all:
// at example.Service$$anonfun$tryResult$1.apply(GetInconsistence.scala:9)
// at example.Service$$anonfun$tryResult$1.apply(GetInconsistence.scala:9)
// at scala.util.Try$.apply(Try.scala:161)
// at example.Service$.tryResult(GetInconsistence.scala:9)
// at example.GetInconsistence$$anonfun$3.apply$mcV$sp(GetInconsistence.scala:50)
} If we classify this inconsistency as a problem introducing a new method with new semantics won't solve the problem: users may still assume consistency and use the old method as before. IMO the long-term solution could be:
I don't suggest wrapping the exception in any case, you can still pattern match on How to deal with the compatibility problems is another topic that needs to be solved but maybe we should agree on the problem first :) |
Otherwise, the stacktrace of the code calling
Failure.get
is lost. Wrapping the Failure exception preserves both stacktraces.This is important because if
Failure.get
is called this is likely to be a bug in the code that callsFailure.get
(and not in the code that produced the original Failure).For some usages of
Try
this may not be too big a problem becauseFailure
creation and callingFailure.get
may occur together so that it is easy enough to infer where the problem is. However, in many casesTry
is used to transport a possibly erroneous result over "bigger distances" where original stacktrace andFailure.get
aren't executed close to each other so that finding the cause can get hard.One particularly irritating effect of the current implementation is that
Await.result(future)
directly relies onTry.get
so that the exception being thrown by that call doesn't include the stacktrace of theAwait.result
call but just the mostly useless stacktrace of the failed Future execution (e.g. a "Future timed out" stack trace that doesn't include any user code).The text was updated successfully, but these errors were encountered: