=htp #19397,#19842 fix content negotiation for non 2xx and Accept handling

akka-http-core/src/main/scala/akka/http/impl/engine/client/PoolSlot.scala

@@ -238,12 +238,6 @@ private object PoolSlot {
     }
   }
 
-  final class UnexpectedDisconnectException(msg: String, cause: Throwable) extends RuntimeException(msg, cause) {
-    def this(msg: String) {
-      this(msg, null)
-    }
-  }
-
   final class UnexpectedDisconnectException(msg: String, cause: Throwable) extends RuntimeException(msg, cause) {
     def this(msg: String) = this(msg, null)
   }

akka-http-testkit/src/main/scala/akka/http/scaladsl/testkit/RouteTest.scala

@@ -4,7 +4,10 @@
 
 package akka.http.scaladsl.testkit
 
+import akka.http.scaladsl.marshalling.ToResponseMarshallable
+import akka.http.scaladsl.server.directives.ExecutionDirectives._
 import akka.http.scaladsl.settings.RoutingSettings
+import akka.stream.impl.ConstantFun
 import com.typesafe.config.{ ConfigFactory, Config }
 import scala.collection.immutable
 import scala.concurrent.{ ExecutionContext, Await, Future }
@@ -111,11 +114,14 @@ trait RouteTest extends RequestBuilding with WSTestRequestBuilding with RouteTes
    * The result of the pipeline is the result that can later be checked with `check`. See the
    * "separate running route from checking" example from ScalatestRouteTestSpec.scala.
    */
-  def runRoute: RouteTestResult ⇒ RouteTestResult = conforms
+  def runRoute: RouteTestResult ⇒ RouteTestResult = ConstantFun.scalaIdentityFunction
 
   // there is already an implicit class WithTransformation in scope (inherited from akka.http.scaladsl.testkit.TransformerPipelineSupport)
   // however, this one takes precedence
   implicit class WithTransformation2(request: HttpRequest) {
+    /**
+     * Apply request to given routes for further inspection in `check { }` block.
+     */
     def ~>[A, B](f: A ⇒ B)(implicit ta: TildeArrow[A, B]): ta.Out = ta(request, f)
   }
 
@@ -152,7 +158,7 @@ trait RouteTest extends RequestBuilding with WSTestRequestBuilding with RouteTes
           val ctx = new RequestContextImpl(effectiveRequest, routingLog.requestLog(effectiveRequest), routingSettings)
           val sealedExceptionHandler = ExceptionHandler.seal(exceptionHandler)
           val semiSealedRoute = // sealed for exceptions but not for rejections
-            Directives.handleExceptions(sealedExceptionHandler) { route }
+            Directives.handleExceptions(sealedExceptionHandler)(route)
           val deferrableRouteResult = semiSealedRoute(ctx)
           deferrableRouteResult.fast.foreach(routeTestResult.handleResult)(executionContext)
           routeTestResult

akka-http-tests/src/test/scala/akka/http/scaladsl/marshalling/ContentNegotiationGivenResponseCodeSpec.scala

@@ -0,0 +1,56 @@
+/*
+ * Copyright (C) 2009-2016 Typesafe Inc. <http://www.typesafe.com>
+ */
+
+package akka.http.scaladsl.marshalling
+
+import akka.http.scaladsl.model.StatusCodes._
+import akka.http.scaladsl.model.headers.Accept
+import akka.http.scaladsl.model.{ ContentTypes, MediaRanges }
+import akka.http.scaladsl.server.{ Route, RoutingSpec }
+
+class ContentNegotiationGivenResponseCodeSpec extends RoutingSpec {
+
+  val routes = {
+    pathPrefix(Segment) { mode ⇒
+      complete {
+        mode match {
+          case "200-text" ⇒ OK -> "ok"
+          case "201-text" ⇒ Created -> "created"
+          case "400-text" ⇒ BadRequest -> "bad-request"
+        }
+      }
+    }
+  }
+
+  "Return NotAcceptable for" should {
+    "200 OK response, when entity not available in Accept-ed MediaRange" in {
+      val request = Post("/200-text").addHeader(Accept(MediaRanges.`application/*`))
+
+      request ~> Route.seal(routes) ~> check {
+        status should ===(NotAcceptable)
+        entityAs[String] should include("text/plain")
+      }
+    }
+
+    "201 Created response, when entity not available in Accept-ed MediaRange" in {
+      val request = Post("/201-text").addHeader(Accept(MediaRanges.`application/*`))
+      request ~> Route.seal(routes) ~> check {
+        status should ===(NotAcceptable)
+        entityAs[String] should include("text/plain")
+      }
+    }
+  }
+
+  "Allow not explicitly Accept-ed content type to be returned if response code is non-2xx" should {
+    "400 BadRequest response, when entity not available in Accept-ed MediaRange" in {
+      val request = Post("/400-text").addHeader(Accept(MediaRanges.`application/*`))
+      request ~> Route.seal(routes) ~> check {
+        status should ===(BadRequest)
+        contentType should ===(ContentTypes.`text/plain(UTF-8)`)
+        entityAs[String] should include("bad-request")
+      }
+    }
+  }
+
+}

akka-http/src/main/scala/akka/http/scaladsl/marshalling/Marshal.scala

@@ -9,11 +9,13 @@ import akka.http.scaladsl.server.ContentNegotiator
 import akka.http.scaladsl.model._
 import akka.http.scaladsl.util.FastFuture._
 
+import scala.util.control.NoStackTrace
+
 object Marshal {
   def apply[T](value: T): Marshal[T] = new Marshal(value)
 
-  case class UnacceptableResponseContentTypeException(supported: Set[ContentNegotiator.Alternative])
-    extends RuntimeException
+  final case class UnacceptableResponseContentTypeException(supported: Set[ContentNegotiator.Alternative])
+    extends RuntimeException with NoStackTrace
 }
 
 class Marshal[A](val value: A) {

akka-http/src/main/scala/akka/http/scaladsl/server/RequestContextImpl.scala

@@ -37,10 +37,11 @@ private[http] class RequestContextImpl(
   override def complete(trm: ToResponseMarshallable): Future[RouteResult] =
     trm(request)(executionContext)
       .fast.map(res ⇒ RouteResult.Complete(res))(executionContext)
-      .fast.recover {
+      .fast.recoverWith {
         case Marshal.UnacceptableResponseContentTypeException(supported) ⇒
-          RouteResult.Rejected(UnacceptedResponseContentTypeRejection(supported) :: Nil)
-        case RejectionError(rej) ⇒ RouteResult.Rejected(rej :: Nil)
+          attemptRecoveryFromUnacceptableResponseContentTypeException(trm, supported)
+        case RejectionError(rej) ⇒
+          Future.successful(RouteResult.Rejected(rej :: Nil))
       }(executionContext)
 
   override def reject(rejections: Rejection*): Future[RouteResult] =
@@ -89,6 +90,13 @@ private[http] class RequestContextImpl(
     case _ ⇒ this
   }
 
+  /** Attempts recovering from the special case when non-2xx response is sent, yet content negotiation was unable to find a match. */
+  private def attemptRecoveryFromUnacceptableResponseContentTypeException(trm: ToResponseMarshallable, supported: Set[ContentNegotiator.Alternative]): Future[RouteResult] =
+    trm.value match {
+      case (status: StatusCode, value) if !status.isSuccess ⇒ this.withAcceptAll.complete(trm) // retry giving up content negotiation
+      case _ ⇒ Future.successful(RouteResult.Rejected(UnacceptedResponseContentTypeRejection(supported) :: Nil))
+    }
+
   private def copy(request: HttpRequest = request,
                    unmatchedPath: Uri.Path = unmatchedPath,
                    executionContext: ExecutionContextExecutor = executionContext,

project/MiMa.scala

@@ -677,7 +677,10 @@ object MiMa extends AutoPlugin {
         ProblemFilters.exclude[ReversedMissingMethodProblem]("akka.http.scaladsl.model.ResponseEntity.withoutSizeLimit"),
 
         // #20014 should have been final always
-        ProblemFilters.exclude[FinalClassProblem]("akka.http.scaladsl.model.EntityStreamSizeException")
+        ProblemFilters.exclude[FinalClassProblem]("akka.http.scaladsl.model.EntityStreamSizeException"),
+
+        // #19849 content negotiation fixes
+        ProblemFilters.exclude[FinalClassProblem]("akka.http.scaladsl.marshalling.Marshal$UnacceptableResponseContentTypeException")
       )
     )
   }