Skip to content

Commit

Permalink
Merge pull request #11305 from mkurz/DevHttpErrorHandler
Browse files Browse the repository at this point in the history
[2.8.x] Introduce DevHttpErrorHandler
  • Loading branch information
gmethvin committed Jun 2, 2022
2 parents f6722f1 + 200a86a commit 296784b
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 19 deletions.
Expand Up @@ -4,10 +4,8 @@

package play.it.views

import play.api.Configuration
import play.api.Environment
import play.api.Mode
import play.api.http.DefaultHttpErrorHandler
import play.api.http.DevHttpErrorHandler
import play.api.test._

class DevErrorPageSpec extends PlaySpecification {
Expand All @@ -20,8 +18,8 @@ class DevErrorPageSpec extends PlaySpecification {
}

"link the error line if play.editor is configured" in {
DefaultHttpErrorHandler.setPlayEditor("someEditorLinkWith %s:%s")
val result = DefaultHttpErrorHandler.onServerError(FakeRequest(), testExceptionSource)
DevHttpErrorHandler.setPlayEditor("someEditorLinkWith %s:%s")
val result = DevHttpErrorHandler.onServerError(FakeRequest(), testExceptionSource)
contentAsString(result) must contain("""href="someEditorLinkWith someSourceFile:100" """)
}

Expand Down
11 changes: 10 additions & 1 deletion core/play/src/main/scala/play/api/http/HttpErrorHandler.scala
Expand Up @@ -521,7 +521,7 @@ class JsonHttpErrorHandler(environment: Environment, sourceMapper: Option[Source
* Note: this HttpErrorHandler should ONLY be used in DEV or TEST. The way this displays errors to the user is
* generally not suitable for a production environment.
*/
object DefaultHttpErrorHandler
object DevHttpErrorHandler
extends DefaultHttpErrorHandler(HttpErrorConfig(showDevErrors = true, playEditor = None), None, None) {
private val logger = Logger(getClass)
private lazy val setEditor: Unit =
Expand All @@ -545,6 +545,15 @@ object DefaultHttpErrorHandler
}
}

/**
* A fallback default HTTP error handler that can be used when there's no application available.
*
* Note: this HttpErrorHandler uses the default `HttpErrorConfig`, which does not `showDevErrors`.
* It is largely here to preserve binary compatibility, but should be overridden with an injected
* HttpErrorHandler.
*/
object DefaultHttpErrorHandler extends DefaultHttpErrorHandler(HttpErrorConfig(), None, None)

/**
* A Java error handler that's provided when a Scala one is configured, so that Java code can still have the error
* handler injected.
Expand Down
6 changes: 3 additions & 3 deletions project/Dependencies.scala
Expand Up @@ -162,10 +162,10 @@ object Dependencies {
sslConfig
) ++ scalaParserCombinators(scalaVersion) ++ specs2Deps.map(_ % Test) ++ javaTestDeps

val nettyVersion = "4.1.75.Final"
val nettyVersion = "4.1.77.Final"

val netty = Seq(
"com.typesafe.netty" % "netty-reactive-streams-http" % "2.0.5",
"com.typesafe.netty" % "netty-reactive-streams-http" % "2.0.6",
("io.netty" % "netty-transport-native-epoll" % nettyVersion).classifier("linux-x86_64")
) ++ specs2Deps.map(_ % Test)

Expand Down Expand Up @@ -288,7 +288,7 @@ object Dependencies {
"com.github.ben-manes.caffeine" % "jcache" % caffeineVersion
) ++ jcacheApi

val playWsStandaloneVersion = "2.1.7"
val playWsStandaloneVersion = "2.1.10"
val playWsDeps = Seq(
"com.typesafe.play" %% "play-ws-standalone" % playWsStandaloneVersion,
"com.typesafe.play" %% "play-ws-standalone-xml" % playWsStandaloneVersion,
Expand Down
2 changes: 1 addition & 1 deletion project/plugins.sbt
Expand Up @@ -5,7 +5,7 @@ enablePlugins(BuildInfoPlugin)
// when updating sbtNativePackager version, be sure to also update the documentation links in
// documentation/manual/working/commonGuide/production/Deploying.md
val sbtNativePackager = "1.5.2"
val mima = "0.8.1"
val mima = "0.9.0"
val sbtJavaAgent = "0.1.5"
val sbtJavaFormatter = "0.5.0"
val sbtJmh = "0.3.7"
Expand Down
Expand Up @@ -29,11 +29,12 @@ import com.typesafe.config.ConfigMemorySize
import javax.net.ssl._
import play.api._
import play.api.http.DefaultHttpErrorHandler
import play.api.http.DevHttpErrorHandler
import play.api.http.HeaderNames
import play.api.http.HttpErrorHandler
import play.api.http.HttpErrorInfo
import play.api.http.{ HttpProtocol => PlayHttpProtocol }
import play.api.http.Status
import play.api.http.{ HttpProtocol => PlayHttpProtocol }
import play.api.internal.libs.concurrent.CoordinatedShutdownSupport
import play.api.libs.streams.Accumulator
import play.api.mvc._
Expand Down Expand Up @@ -292,6 +293,11 @@ class AkkaHttpServer(context: AkkaHttpServer.Context) extends Server {
}
}

private lazy val fallbackErrorHandler = mode match {
case Mode.Prod => DefaultHttpErrorHandler
case _ => DevHttpErrorHandler
}

private def resultUtils(tryApp: Try[Application]): ServerResultUtils =
reloadCache.cachedFrom(tryApp).resultUtils
private def modelConversion(tryApp: Try[Application]): AkkaModelConversion =
Expand All @@ -314,7 +320,7 @@ class AkkaHttpServer(context: AkkaHttpServer.Context) extends Server {
val debugInfo: Option[ServerDebugInfo] = reloadCache.cachedFrom(tryApp).serverDebugInfo
ServerDebugInfo.attachToRequestHeader(convertedRequestHeader, debugInfo)
}
val (taggedRequestHeader, handler) = Server.getHandlerFor(debugInfoRequestHeader, tryApp)
val (taggedRequestHeader, handler) = Server.getHandlerFor(debugInfoRequestHeader, tryApp, fallbackErrorHandler)
val responseFuture = executeHandler(
tryApp,
decodedRequest,
Expand Down Expand Up @@ -345,7 +351,7 @@ class AkkaHttpServer(context: AkkaHttpServer.Context) extends Server {
// Get the app's HttpErrorHandler or fallback to a default value
val errorHandler: HttpErrorHandler = tryApp match {
case Success(app) => app.errorHandler
case Failure(_) => DefaultHttpErrorHandler
case Failure(_) => fallbackErrorHandler
}

// default execution context used for executing the action
Expand Down
Expand Up @@ -20,6 +20,7 @@ import play.api.libs.streams.Accumulator
import play.api.mvc._
import play.api.Application
import play.api.Logger
import play.api.Mode
import play.core.server.NettyServer
import play.core.server.Server
import play.core.server.common.ReloadCache
Expand Down Expand Up @@ -125,7 +126,7 @@ private[play] class PlayRequestHandler(
clientError(Status.REQUEST_ENTITY_TOO_LARGE, "Request Entity Too Large")
} else {
val debugHeader: RequestHeader = attachDebugInfo(untagged)
Server.getHandlerFor(debugHeader, tryApp)
Server.getHandlerFor(debugHeader, tryApp, fallbackErrorHandler)
}
}

Expand Down Expand Up @@ -330,9 +331,14 @@ private[play] class PlayRequestHandler(
private def errorHandler(tryApp: Try[Application]): HttpErrorHandler =
tryApp match {
case Success(app) => app.errorHandler
case Failure(_) => DefaultHttpErrorHandler
case Failure(_) => fallbackErrorHandler
}

private lazy val fallbackErrorHandler = server.mode match {
case Mode.Prod => DefaultHttpErrorHandler
case _ => DevHttpErrorHandler
}

/**
* Sends a simple response with no body, then closes the connection.
*/
Expand Down
Expand Up @@ -5,14 +5,13 @@
package play.core.server

import java.util.function.{ Function => JFunction }

import akka.actor.CoordinatedShutdown
import akka.annotation.ApiMayChange
import com.typesafe.config.Config
import com.typesafe.config.ConfigFactory
import play.api.ApplicationLoader.Context
import play.api._
import play.api.http.DefaultHttpErrorHandler
import play.api.http.DevHttpErrorHandler
import play.api.http.HttpErrorHandler
import play.api.http.Port
import play.api.inject.ApplicationLifecycle
Expand Down Expand Up @@ -98,7 +97,11 @@ object Server {
* i.e. if there's an error loading the application.
* - If an exception is thrown.
*/
private[server] def getHandlerFor(request: RequestHeader, tryApp: Try[Application]): (RequestHeader, Handler) = {
private[server] def getHandlerFor(
request: RequestHeader,
tryApp: Try[Application],
fallbackErrorHandler: HttpErrorHandler
): (RequestHeader, Handler) = {
@inline def handleErrors(
errorHandler: HttpErrorHandler,
req: RequestHeader
Expand Down Expand Up @@ -129,7 +132,7 @@ object Server {
handleErrors(application.errorHandler, enrichedRequest)
}
} catch {
handleErrors(DefaultHttpErrorHandler, request)
handleErrors(fallbackErrorHandler, request)
}
}

Expand Down

0 comments on commit 296784b

Please sign in to comment.