Should the future API decide what exceptions are or aren't fatal?

[James Roper]

Let me just start by saying this is not about NonFatal, I think the exceptions that NonFatal does and doesn't consider fatal are fine.  This is about whether the Future API should use NonFatal.

The problem is, why should Futures force a policy of what exceptions are considered fatal and what aren't on consumers of it's API?  If it didn't do this, then consumers would have to do this:

future.onFailure {

  case NonFatal(e) => ...

}

Thus making that decision for themselves.  The above is not only terse, it is also idiomatic, it matches what the consumer would do for an ordinary try catch.  But the Scala future API doesn't allow the user to decide which exceptions are fatal and which aren't, because in all its exception handling, it only catches NonFatal.

Let me give a very good example of when this is bad.  In Play framework, everything is asynchronous, going through futures, and in development mode, the framework acts more like a development environment.  In a development environment, NotImplementedErrors and LinkageErrors are not uncommon.  The development environment should handle them, and report the error to the user, so that the user can fix their code.  And Play tries to do this, when a user changes their code, and hits refresh in their browser, if an exception occurs Play will catch it and render it in their browser, including code context etc.  But since futures don't give Play an opportunity to catch NotImplementedErrors and LinkageErrors, Play can't do this.

I think APIs in the core Scala library should not force policies upon API consumers like this, it should allow API consumers to make the decision of what is a fatal exception and what isn't.  NonFatal itself is a great shorthand for users to use themselves, it shouldn't be use in the Futures library.

[Jason Zaugg]

On Mon, Jan 28, 2013 at 1:13 AM, James Roper <jro...@gmail.com> wrote:

Let me just start by saying this is not about NonFatal, I think the exceptions that NonFatal does and doesn't consider fatal are fine.  This is about whether the Future API should use NonFatal.

 

The problem is, why should Futures force a policy of what exceptions are considered fatal and what aren't on consumers of it's API?  If it didn't do this, then consumers would have to do this:

future.onFailure {

  case NonFatal(e) => ...

}

Thus making that decision for themselves.  The above is not only terse, it is also idiomatic, it matches what the consumer would do for an ordinary try catch.  But the Scala future API doesn't allow the user to decide which exceptions are fatal and which aren't, because in all its exception handling, it only catches NonFatal.

Future and Try are indeed pretty opinionated on this matter. Abstracting over this would complicate the API; catching `_: Throwable` brings  different set of problems, most notably masking platform errors like OutOfMemoryError by translating them into 'user' errors.

If you control the construction of the futures, you could go for something like:

case class DevError(cause: Throwable) extends RuntimeException(cause)

@inline def allowingDevErrors[A](a: => A) {

  if (isDevMode) try a catch { case ex @ (_: NotImplementedError | _: LinkageError) => throw DevError(ex)

  else a

}

future { allowingDevErrors( ... ) }

Would that pattern help you out at all?

-jason

[√iktor Ҡlang]

On Mon, Jan 28, 2013 at 2:50 PM, Jason Zaugg <jza...@gmail.com> wrote:

On Mon, Jan 28, 2013 at 1:13 AM, James Roper <jro...@gmail.com> wrote:

Let me just start by saying this is not about NonFatal, I think the exceptions that NonFatal does and doesn't consider fatal are fine.  This is about whether the Future API should use NonFatal.

 

The problem is, why should Futures force a policy of what exceptions are considered fatal and what aren't on consumers of it's API?  If it didn't do this, then consumers would have to do this:

future.onFailure {

  case NonFatal(e) => ...

}

Thus making that decision for themselves.  The above is not only terse, it is also idiomatic, it matches what the consumer would do for an ordinary try catch.  But the Scala future API doesn't allow the user to decide which exceptions are fatal and which aren't, because in all its exception handling, it only catches NonFatal.

Future and Try are indeed pretty opinionated on this matter. Abstracting over this would complicate the API; catching `_: Throwable` brings  different set of problems, most notably masking platform errors like OutOfMemoryError by translating them into 'user' errors.

Or, is NotImplementedError really an Error? And if so, is it Fatal?

 

If you control the construction of the futures, you could go for something like:

case class DevError(cause: Throwable) extends RuntimeException(cause)

@inline def allowingDevErrors[A](a: => A) {

  if (isDevMode) try a catch { case ex @ (_: NotImplementedError | _: LinkageError) => throw DevError(ex)

  else a

}

future { allowingDevErrors( ... ) }

Would that pattern help you out at all?

-jason

--
You received this message because you are subscribed to the Google Groups "scala-user" group.
To unsubscribe from this group, send email to scala-user+...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
 
 

--

Viktor Klang
Director of Engineering

Typesafe - The software stack for applications that scale
Twitter: @viktorklang

[Simon Ochsenreither]

I think the point is that this can go horribly wrong if people are not aware of the requirements, especially with ControlThrowable.
If the issues with using a user-catchable exception for control flow could be fixed, imho a change could alt least be considered, but in the current situation ...

[√iktor Ҡlang]

On Mon, Jan 28, 2013 at 5:17 PM, Simon Ochsenreither <simon.och...@gmail.com> wrote:

I think the point is that this can go horribly wrong if people are not aware of the requirements, especially with ControlThrowable.
If the issues with using a user-catchable exception for control flow could be fixed, imho a change could alt least be considered, but in the current situation ...

I'm not sure I follow, elaborate?

Cheers,

√

 

--
You received this message because you are subscribed to the Google Groups "scala-user" group.
To unsubscribe from this group, send email to scala-user+...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
 
 

[Ismael Juma]

On Mon, Jan 28, 2013 at 1:50 PM, Jason Zaugg <jza...@gmail.com> wrote:

Future and Try are indeed pretty opinionated on this matter. Abstracting over this would complicate the API; catching `_: Throwable` brings  different set of problems, most notably masking platform errors like OutOfMemoryError by translating them into 'user' errors.

OutOfMemoryError can also be a 'user' error. As well as StackOverflowError. ControlThrowable instances should never be though. I am not yet sure if treating them the same is the right thing for APIs that serve as building blocks, personally.

Best,

Ismael

[James Roper]

On Tuesday, 29 January 2013 00:50:28 UTC+11, Jason Zaugg wrote:

On Mon, Jan 28, 2013 at 1:13 AM, James Roper <jro...@gmail.com> wrote:

Let me just start by saying this is not about NonFatal, I think the exceptions that NonFatal does and doesn't consider fatal are fine.  This is about whether the Future API should use NonFatal.

 

The problem is, why should Futures force a policy of what exceptions are considered fatal and what aren't on consumers of it's API?  If it didn't do this, then consumers would have to do this:

future.onFailure {

  case NonFatal(e) => ...

}

Thus making that decision for themselves.  The above is not only terse, it is also idiomatic, it matches what the consumer would do for an ordinary try catch.  But the Scala future API doesn't allow the user to decide which exceptions are fatal and which aren't, because in all its exception handling, it only catches NonFatal.

Future and Try are indeed pretty opinionated on this matter. Abstracting over this would complicate the API; catching `_: Throwable` brings  different set of problems, most notably masking platform errors like OutOfMemoryError by translating them into 'user' errors.

OOME is always a funny one, it usually is a user error, but handling it can be very dangerous (ie, can exacerbate the problem, can cause the JVM to just hang, etc).  One way to deal with this is to have an oome parachute, ie, hold a memory buffer somewhere that you release when you encounter an OOME, and hope that that's going to give you enough heap space, and that you'll get to it first, to be able to handle the error.  But that's probably overkill for this situation.  But StackOverflowError is similarly dangerous to handle, since handling it may produce another StackOverflowError (as we've seen recently with futures), yet futures still try to handle that.  Another one is ThreadDeath, which if handled, must always be rethrown.  UnknownError is probably also not a good idea to handle.

But I definitely think LinkageError and NotImplementedError are safe for futures to handle - you're not going to encounter them in a running production system, that is, you'll only encounter them soon after the production system starts up.  You could encounter them if you were doing dynamic class loading, but in that case you probably also want to handle them.

[√iktor Ҡlang]

On Mon, Jan 28, 2013 at 10:51 PM, James Roper <jro...@gmail.com> wrote:

On Tuesday, 29 January 2013 00:50:28 UTC+11, Jason Zaugg wrote:

On Mon, Jan 28, 2013 at 1:13 AM, James Roper <jro...@gmail.com> wrote:

Let me just start by saying this is not about NonFatal, I think the exceptions that NonFatal does and doesn't consider fatal are fine.  This is about whether the Future API should use NonFatal.

 

The problem is, why should Futures force a policy of what exceptions are considered fatal and what aren't on consumers of it's API?  If it didn't do this, then consumers would have to do this:

future.onFailure {

  case NonFatal(e) => ...

}

Thus making that decision for themselves.  The above is not only terse, it is also idiomatic, it matches what the consumer would do for an ordinary try catch.  But the Scala future API doesn't allow the user to decide which exceptions are fatal and which aren't, because in all its exception handling, it only catches NonFatal.

Future and Try are indeed pretty opinionated on this matter. Abstracting over this would complicate the API; catching `_: Throwable` brings  different set of problems, most notably masking platform errors like OutOfMemoryError by translating them into 'user' errors.

OOME is always a funny one, it usually is a user error, but handling it can be very dangerous (ie, can exacerbate the problem, can cause the JVM to just hang, etc).  One way to deal with this is to have an oome parachute, ie, hold a memory buffer somewhere that you release when you encounter an OOME, and hope that that's going to give you enough heap space, and that you'll get to it first, to be able to handle the error.  But that's probably overkill for this situation.  But StackOverflowError is similarly dangerous to handle, since handling it may produce another StackOverflowError (as we've seen recently with futures), yet futures still try to handle that.  Another one is ThreadDeath, which if handled, must always be rethrown.  UnknownError is probably also not a good idea to handle.

Yes. StackOverflowError isn't considered Fatal though.

 

But I definitely think LinkageError and NotImplementedError are safe for futures to handle -

I disagree wrt LinkageError – check direct subclasses of LinkageError: http://docs.oracle.com/javase/7/docs/api/java/lang/LinkageError.html

 

you're not going to encounter them in a running production system, that is, you'll only encounter them soon after the production system starts up.  You could encounter them if you were doing dynamic class loading, but in that case you probably also want to handle them. 

The only things here which I think are not necessarily fatal are NotImplementedError and ControlThrowable. So I could argue that they should be removed from there.

Cheers,

√

 

 

If you control the construction of the futures, you could go for something like:

case class DevError(cause: Throwable) extends RuntimeException(cause)

@inline def allowingDevErrors[A](a: => A) {

  if (isDevMode) try a catch { case ex @ (_: NotImplementedError | _: LinkageError) => throw DevError(ex)

  else a

}

future { allowingDevErrors( ... ) }

Would that pattern help you out at all?

-jason

--
You received this message because you are subscribed to the Google Groups "scala-user" group.

To unsubscribe from this group and stop receiving emails from it, send an email to scala-user+...@googlegroups.com.

For more options, visit https://groups.google.com/groups/opt_out.
 
 

[Rex Kerr]

On Mon, Jan 28, 2013 at 5:32 PM, √iktor Ҡlang <viktor...@gmail.com> wrote:

On Mon, Jan 28, 2013 at 10:51 PM, James Roper <jro...@gmail.com> wrote:

On Tuesday, 29 January 2013 00:50:28 UTC+11, Jason Zaugg wrote:

On Mon, Jan 28, 2013 at 1:13 AM, James Roper <jro...@gmail.com> wrote:

Let me just start by saying this is not about NonFatal, I think the exceptions that NonFatal does and doesn't consider fatal are fine.  This is about whether the Future API should use NonFatal.

 

The problem is, why should Futures force a policy of what exceptions are considered fatal and what aren't on consumers of it's API?  If it didn't do this, then consumers would have to do this:

future.onFailure {

  case NonFatal(e) => ...

}

Thus making that decision for themselves.  The above is not only terse, it is also idiomatic, it matches what the consumer would do for an ordinary try catch.  But the Scala future API doesn't allow the user to decide which exceptions are fatal and which aren't, because in all its exception handling, it only catches NonFatal.

Future and Try are indeed pretty opinionated on this matter. Abstracting over this would complicate the API; catching `_: Throwable` brings  different set of problems, most notably masking platform errors like OutOfMemoryError by translating them into 'user' errors.

OOME is always a funny one, it usually is a user error, but handling it can be very dangerous (ie, can exacerbate the problem, can cause the JVM to just hang, etc).  One way to deal with this is to have an oome parachute, ie, hold a memory buffer somewhere that you release when you encounter an OOME, and hope that that's going to give you enough heap space, and that you'll get to it first, to be able to handle the error.  But that's probably overkill for this situation.  But StackOverflowError is similarly dangerous to handle, since handling it may produce another StackOverflowError (as we've seen recently with futures), yet futures still try to handle that.  Another one is ThreadDeath, which if handled, must always be rethrown.  UnknownError is probably also not a good idea to handle.

Yes. StackOverflowError isn't considered Fatal though.

 

But I definitely think LinkageError and NotImplementedError are safe for futures to handle -

I disagree wrt LinkageError – check direct subclasses of LinkageError: http://docs.oracle.com/javase/7/docs/api/java/lang/LinkageError.html

 

you're not going to encounter them in a running production system, that is, you'll only encounter them soon after the production system starts up.  You could encounter them if you were doing dynamic class loading, but in that case you probably also want to handle them. 

The only things here which I think are not necessarily fatal are NotImplementedError and ControlThrowable. So I could argue that they should be removed from there.

NonFatal is not only used in futures, though.  You could argue that futures should catch it, but not that Try should since that would yield bugs in cases like

    breakable {

      for (s <- ss) Try{ val i = s.toInt; if (i<0) { break }; i }
      ...
    }

So if futures do need to catch ControlThrowable, they'd better do it with a different construct than non-futures do.

  --Rex

P.S. Switching from the unapply form to the boolean form `case e if NonFatal(e) => ` would make it easier to do pluggable exception handlers if one wanted to do that; you wouldn't need to wrap a Throwable => Boolean in something that supplies an unapply.

[√iktor Ҡlang]

I think the current NonFatal conflates 2 distinctly different things:

1) Throwables that are to be considered _fatal_ i.e. that the program needs termination

2) Throwables that user code is not intended to catch

I propose to split out number 2 into a different entity called ``Catchable`` or otherwise, and review where NonFatal is currently used in a "flawed" way.

Cheers,

√

[Jason Zaugg]

On Tue, Jan 29, 2013 at 11:16 AM, √iktor Ҡlang <viktor...@gmail.com> wrote:

I think the current NonFatal conflates 2 distinctly different things:

1) Throwables that are to be considered _fatal_ i.e. that the program needs termination

2) Throwables that user code is not intended to catch

I propose to split out number 2 into a different entity called ``Catchable`` or otherwise, and review where NonFatal is currently used in a "flawed" way.

I'd just like to point out that changing the behaviour of NonFatal (and, by extension, Try and Future) is just about impossible to do without silently breaking someone's code.

-jason

[Alec Zorab]

... you're not suggesting that the inclusion of Try wasn't fully thought out, are you?

--

[√iktor Ҡlang]

Well, right now it's actually silently broken for NotImplementedError and for ControlThrowable. Can we try to enumerate where it [the change] would actually _cause_ problems?

Cheers,

√

 

-jason

[Jason Zaugg]

On Tue, Jan 29, 2013 at 11:26 AM, Alec Zorab <alec...@gmail.com> wrote:

... you're not suggesting that the inclusion of Try wasn't fully thought out, are you?

I don't think that Try itself is broken, it is deliberately opinionated about what it catches. If it wasn't, it would be difficult to combine different Try flavours together.

My point is a general one: the inclusion of *any* code in the standard library makes it very difficult to change. 

-jason

[Jason Zaugg]

On Tue, Jan 29, 2013 at 11:30 AM, √iktor Ҡlang <viktor...@gmail.com> wrote:

Well, right now it's actually silently broken for NotImplementedError and for ControlThrowable.

Could you elaborate on that assertion with some code?

-jason

[Rex Kerr]

This probably will not do what one wants:

def root(x: Double): Double = 
  Await.result( Future{ if (x<0) return Double.NaN else math.sqrt(x) }, Duration.Inf)

Evidence:

scala> root(2)
res36: Double = 1.4142135623730951

scala> root(-1)

//...still waiting....

Not so cool.

Try is not broken, but Future is.

  --Rex

[√iktor Ҡlang]

import scala.concurrent.ExecutionContext.Implicits.global

val f = Future(???)

I think the guideline of the case where NonFatal(e) == false is that it is best to shut the JVM down.

I don't think that's true for NotImplementedError or for ControlThrowable or for InterruptedException, they all mean essentially "just don't catch me bro".

object Catchable {

def apply(t: Throwable): Boolean = t match {
  case _: InterruptedException _: ControlThrowable | _: NotImplementedError => false
  case _ => NonFatal(t)
}

}

 

-jason

[Naftoli Gugenheim]

Why not make it configurable? For instance every method that matches on NonFatal could allow the extractor to be passed implicitly.

--

[√iktor Ҡlang]

On Tue, Jan 29, 2013 at 11:55 AM, Naftoli Gugenheim <nafto...@gmail.com> wrote:

Why not make it configurable? For instance every method that matches on NonFatal could allow the extractor to be passed implicitly.

How would that help in this case?

Cheers,

√

[Naftoli Gugenheim]

(a) The subject -- it would be a way to allow individuals to decide, or include additional exceptions

(b) Backward compatibility -- it would allow to fix the default implementation while allowing people relying on the existing behavior to keep it

(c) Differences of opinion -- if there are any exceptions that are not black-and-white in one category or the other, it would allow it to be configured.

[√iktor Ҡlang]

On Tue, Jan 29, 2013 at 12:14 PM, Naftoli Gugenheim <nafto...@gmail.com> wrote:

(a) The subject -- it would be a way to allow individuals to decide, or include additional exceptions

(b) Backward compatibility -- it would allow to fix the default implementation while allowing people relying on the existing behavior to keep it

(c) Differences of opinion -- if there are any exceptions that are not black-and-white in one category or the other, it would allow it to be configured.

I don't understand how that solves the issue with: val f = Future(???)

[Naftoli Gugenheim]

I wasn't responding specifically to that. Can you clarify what the issue is?

[√iktor Ҡlang]

On Tue, Jan 29, 2013 at 12:21 PM, Naftoli Gugenheim <nafto...@gmail.com> wrote:

I wasn't responding specifically to that. Can you clarify what the issue is?

NotImplementedError is considered fatal by NonFatal, which means that Futures will just rethrow it, since it's fatal.

But it's mixing concerns, fatality and catchability are orthogonal, and I think that should be encoded in separate objects.

The matter gets complicated due to FJP not invoking the UEH for adapted runnables being adapted by FJP.execute().

For more background, see: https://issues.scala-lang.org/browse/SI-7029?focusedCommentId=62487#comment-62487