Browse files

Merge pull request #345 from julienrf/fix/496

Handle runtime errors of async results
  • Loading branch information...
2 parents bd62be9 + 85aedbf commit def888333ea435437edb7f70ca3b7f48877af1c7 @pk11 pk11 committed Jun 4, 2012
View
39 framework/src/play/src/main/scala/play/api/Application.scala
@@ -131,6 +131,45 @@ class Application(val path: File, val classloader: ClassLoader, val sources: Opt
}
+ /**
+ * Handle a runtime error during the execution of an action
+ */
+ private[play] def handleError(request: RequestHeader, e: Throwable): Result = try {
+ e match {
+ case e: PlayException.UsefulException => throw e
+ case e: Throwable => {
+
+ val source = sources.flatMap(_.sourceFor(e))
+
+ throw new PlayException(
+ "Execution exception",
+ "[%s: %s]".format(e.getClass.getSimpleName, e.getMessage),
+ Some(e)) with PlayException.ExceptionSource {
+ def line = source.map(_._2)
+ def position = None
+ def input = source.map(_._1).map(scalax.file.Path(_))
+ def sourceName = source.map(_._1.getAbsolutePath)
+ }
+ }
+ }
+ } catch {
+ case e => try {
+ Logger.error(
+ """
+ |
+ |! %sInternal server error, for request [%s] ->
+ |""".stripMargin.format(e match {
+ case p: PlayException => "@" + p.id + " - "
+ case _ => ""
+ }, request),
+ e)
+
+ global.onError(request, e)
+ } catch {
+ case e => DefaultGlobal.onError(request, e)
+ }
+ }
+
private[api] def pluginClasses: Seq[String] = {
import scalax.file._
View
6 framework/src/play/src/main/scala/play/core/server/netty/PlayDefaultUpstreamHandler.scala
@@ -92,8 +92,10 @@ private[server] class PlayDefaultUpstreamHandler(server: Server, allChannels: De
case AsyncResult(p) => p.extend1 {
case Redeemed(v) => handle(v)
case Thrown(e) => {
- Logger("play").error("Waiting for a promise, but got an error: " + e.getMessage, e)
- handle(Results.InternalServerError)
+ server.applicationProvider.get match {
+ case Right(app) => handle(app.handleError(requestHeader, e))
+ case Left(_) => handle(Results.InternalServerError)
+ }
}
}
View
40 framework/src/play/src/main/scala/play/core/system/Invoker.scala
@@ -114,45 +114,11 @@ class ActionInvoker extends Actor {
case Invoker.HandleAction(request, response: Response, action, app: Application) => {
val result = try {
- try {
- Threads.withContextClassLoader(app.classloader) {
- action(request)
- }
- } catch {
- case e: PlayException.UsefulException => throw e
- case e: Throwable => {
-
- val source = app.sources.flatMap(_.sourceFor(e))
-
- throw new PlayException(
- "Execution exception",
- "[%s: %s]".format(e.getClass.getSimpleName, e.getMessage),
- Some(e)) with PlayException.ExceptionSource {
- def line = source.map(_._2)
- def position = None
- def input = source.map(_._1).map(scalax.file.Path(_))
- def sourceName = source.map(_._1.getAbsolutePath)
- }
-
- }
+ Threads.withContextClassLoader(app.classloader) {
+ action(request)
}
} catch {
- case e => try {
-
- Logger.error(
- """
- |
- |! %sInternal server error, for request [%s] ->
- |""".stripMargin.format(e match {
- case p: PlayException => "@" + p.id + " - "
- case _ => ""
- }, request),
- e)
-
- app.global.onError(request, e)
- } catch {
- case e => DefaultGlobal.onError(request, e)
- }
+ case e => app.handleError(request, e)
}
response.handle {
View
11 framework/test/integrationtest/app/Global.scala
@@ -0,0 +1,11 @@
+
+import play.api.GlobalSettings
+import play.api.mvc.{RequestHeader, Result}
+import play.api.mvc.Results.InternalServerError
+import play.api.Logger
+
+object Global extends GlobalSettings {
+ override def onError(r: RequestHeader, e: Throwable): Result = {
+ InternalServerError("Something went wrong.")
+ }
+}
View
12 framework/test/integrationtest/app/controllers/Application.scala
@@ -8,6 +8,7 @@ import play.api.cache.Cache
import play.api.libs.json._
import play.api.libs.json.Json._
import play.api.libs.Jsonp
+import play.api.libs.concurrent.Promise
import models._
import models.Protocol._
@@ -117,4 +118,15 @@ object Application extends Controller {
val file = new File(filepath)
Ok.sendFile(file, onClose = () => { file.delete() })
}
+
+ def syncError = Action {
+ sys.error("Error")
+ Ok
+ }
+
+ def asyncError = Action {
+ Async {
+ Promise.pure[Result](sys.error("Error"))
+ }
+ }
}
View
3 framework/test/integrationtest/conf/routes
@@ -44,6 +44,9 @@ GET /accept-java controllers.JavaApi.accept()
GET /onCloseSendFile/*fp controllers.Application.onCloseSendFile(fp)
+GET /sync-error controllers.Application.syncError()
+GET /async-error controllers.Application.asyncError()
+
# Map static resources from the /public folder to the /public path
GET /public/*file controllers.Assets.at(path="/public", file)
View
15 framework/test/integrationtest/test/FunctionalSpec.scala
@@ -148,6 +148,21 @@ class FunctionalSpec extends Specification {
}
}
+ "Provide a hook to handle errors" in {
+ "Synchronous results" in {
+ running(TestServer(9000), HTMLUNIT) { browser =>
+ browser.goTo("http://localhost:9000/sync-error")
+ browser.pageSource must equalTo ("Something went wrong.")
+ }
+ }
+ "Asynchronous results" in {
+ running(TestServer(9000), HTMLUNIT) { browser =>
+ browser.goTo("http://localhost:9000/async-error")
+ browser.pageSource must equalTo ("Something went wrong.")
+ }
+ }
+ }
+
}
}

0 comments on commit def8883

Please sign in to comment.