Skip to content
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

Ability to access current response timeout (#1811) #1828

Merged
merged 6 commits into from
Mar 6, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions akka-http-core/src/main/java/akka/http/javadsl/TimeoutAccess.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,30 @@

package akka.http.javadsl;

import akka.annotation.DoNotInherit;
import akka.http.javadsl.model.HttpRequest;
import akka.http.javadsl.model.HttpResponse;
import akka.japi.Function;
import scala.concurrent.duration.Duration;

/**
* Enables programmatic access to the server-side request timeout logic.
*
* Not for user extension.
*/
@DoNotInherit
public interface TimeoutAccess {

/**
* Returns the currently set timeout.
* The timeout period is measured as of the point in time that the end of the request has been received,
* which may be in the past or in the future!
*
* Due to the inherent raciness it is not guaranteed that the returned timeout was applied before
* the previously set timeout has expired!
*/
Duration getTimeout();

/**
* Tries to set a new timeout.
* The timeout period is measured as of the point in time that the end of the request has been received,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,3 +74,7 @@ ProblemFilters.exclude[ReversedMissingMethodProblem]("akka.http.scaladsl.model.H

# HttpExt constructor should only be called from Http extension
ProblemFilters.exclude[IncompatibleMethTypeProblem]("akka.http.scaladsl.HttpExt.this")

# Method additions to @DoNotInherit class
ProblemFilters.exclude[ReversedMissingMethodProblem]("akka.http.javadsl.TimeoutAccess.getTimeout")
ProblemFilters.exclude[ReversedMissingMethodProblem]("akka.http.scaladsl.TimeoutAccess.timeout")
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,8 @@ private[http] object HttpServerBluePrint {
extends AtomicReference[Future[TimeoutSetup]] with TimeoutAccess with (HttpRequest ⇒ HttpResponse) { self ⇒
import materializer.executionContext

private var currentTimeout = initialTimeout

initialTimeout match {
case timeout: FiniteDuration ⇒ set {
requestEnd.fast.map(_ ⇒ new TimeoutSetup(Deadline.now, schedule(timeout, this), timeout, this))
Expand Down Expand Up @@ -340,6 +342,7 @@ private[http] object HttpServerBluePrint {
case x: FiniteDuration ⇒ schedule(old.timeoutBase + x - Deadline.now, newHandler)
case _ ⇒ null // don't schedule a new timeout
}
currentTimeout = newTimeout
new TimeoutSetup(old.timeoutBase, newScheduling, newTimeout, newHandler)
} else old // too late, the previously set timeout cannot be cancelled anymore
}
Expand All @@ -353,6 +356,8 @@ private[http] object HttpServerBluePrint {
update(timeout, handler(_: HttpRequest).asScala)
def updateHandler(handler: Function[model.HttpRequest, model.HttpResponse]): Unit =
updateHandler(handler(_: HttpRequest).asScala)

def timeout = currentTimeout
}

class ControllerStage(settings: ServerSettings, log: LoggingAdapter)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,32 @@

package akka.http.scaladsl

import akka.annotation.DoNotInherit

import scala.concurrent.duration.Duration
import akka.http.scaladsl.model.{ HttpResponse, HttpRequest }
import akka.http.scaladsl.model.{ HttpRequest, HttpResponse }

/**
* Enables programmatic access to the server-side request timeout logic.
*
* Not for user extension.
*/
@DoNotInherit
trait TimeoutAccess extends akka.http.javadsl.TimeoutAccess {

/**
* Returns the currently set timeout.
* The timeout period is measured as of the point in time that the end of the request has been received,
* which may be in the past or in the future!
*
* Due to the inherent raciness it is not guaranteed that the returned timeout was applied before
* the previously set timeout has expired!
*/
def timeout: Duration

/** Java API */
def getTimeout: Duration = timeout

/**
* Tries to set a new timeout.
* The timeout period is measured as of the point in time that the end of the request has been received,
Expand Down
5 changes: 4 additions & 1 deletion akka-http/src/main/mima-filters/10.0.11.backwards.excludes
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,7 @@ ProblemFilters.exclude[DirectMissingMethodProblem]("akka.http.scaladsl.coding.No
ProblemFilters.exclude[IncompatibleMethTypeProblem]("akka.http.scaladsl.coding.NoCoding.decode")
ProblemFilters.exclude[DirectMissingMethodProblem]("akka.http.scaladsl.coding.Encoder.encode")
ProblemFilters.exclude[DirectMissingMethodProblem]("akka.http.scaladsl.coding.Decoder.decode")
ProblemFilters.exclude[IncompatibleMethTypeProblem]("akka.http.scaladsl.coding.Decoder.decode")
ProblemFilters.exclude[IncompatibleMethTypeProblem]("akka.http.scaladsl.coding.Decoder.decode")

# Additions to @DoNotInherit directives trait
ProblemFilters.exclude[ReversedMissingMethodProblem]("akka.http.scaladsl.server.directives.TimeoutDirectives.extractRequestTimeout")
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ package akka.http.javadsl.server.directives

import java.util.function.{ Function ⇒ JFunction, Supplier }

import scala.concurrent.duration.Duration

import akka.http.javadsl.model.{ HttpRequest, HttpResponse }
import akka.http.javadsl.server.Route
import akka.http.scaladsl.server.{ Directives ⇒ D }
Expand All @@ -14,6 +16,10 @@ import akka.http.impl.util.JavaMapping.Implicits._

abstract class TimeoutDirectives extends WebSocketDirectives {

def extractRequestTimeout(inner: JFunction[Duration, Route]): RouteAdapter = RouteAdapter {
D.extractRequestTimeout { timeout ⇒ inner.apply(timeout).delegate }
}

/**
* Tries to set a new request timeout and handler (if provided) at the same time.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ package akka.http.scaladsl.server.directives

import akka.http.scaladsl.model._
import akka.http.scaladsl.model.headers.`Timeout-Access`
import akka.http.scaladsl.server.{ Directive, Directive0 }

import akka.http.scaladsl.server.{ Directive, Directive0, Directive1 }
import scala.concurrent.duration.Duration

/**
Expand All @@ -16,6 +15,23 @@ import scala.concurrent.duration.Duration
*/
trait TimeoutDirectives {

/**
* Return the currently set request timeout.
*
* Note that this may be changed in inner directives.
*
* @group timeout
*/
def extractRequestTimeout: Directive1[Duration] = Directive { inner ⇒ ctx ⇒
val timeout = ctx.request.header[`Timeout-Access`] match {
case Some(t) ⇒ t.timeoutAccess.getTimeout
case _ ⇒
ctx.log.warning("extractRequestTimeout was used in route however no request-timeout is set!")
Duration.Inf
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyone have input on how this error case should be handled?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duration.Inf is fine, no need to log a warning. Let's document it on the directive.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it a common case that the timeout access header is missing? What situation should we tell users to expect it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, good point. I think there's currently no way the header would be missing. Maybe we can just fail in that case because it should be seldom enough?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think failing or continuing with Duration.Inf would both be fine, maybe bump the log to error level.
What would failure look like here? Completing with an exception?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe, let's keep it for now. We might want to allow switching off request timeouts completely for performance reasons later on, so not requiring that this header is always available is maybe the best solution.

}
inner(Tuple1(timeout))(ctx)
}

/**
* @group timeout
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# extractRequestTimeout

@@@ div { .group-scala }

## Signature

@@signature [TimeoutDirectives.scala]($akka-http$/akka-http/src/main/scala/akka/http/scaladsl/server/directives/TimeoutDirectives.scala) { #extractRequestTimeout }

@@@

## Description

This directive extracts the currently set request timeout.

@@@ warning
Please note that this extracts the request timeout at the current moment, but the timeout can be changed concurrently.
See other timeout directives about raciness inherent to timeout directives.
@@@

For more information about various timeouts in Akka HTTP see @ref[Akka HTTP Timeouts](../../../common/timeouts.md).

## Example

Scala
: @@snip [TimeoutDirectivesExamplesSpec.scala]($test$/scala/docs/http/scaladsl/server/directives/TimeoutDirectivesExamplesSpec.scala) { #extractRequestTimeout }

Java
: @@snip [TimeoutDirectivesExamplesTest.java]($test$/java/docs/http/javadsl/server/directives/TimeoutDirectivesExamplesTest.java) { #extractRequestTimeout }
Original file line number Diff line number Diff line change
Expand Up @@ -177,4 +177,29 @@ public void testRequestTimeoutCustomResponseCanBeAddedSeparately() {
assert (StatusCodes.ENHANCE_YOUR_CALM.equals(statusCode));
//#withRequestTimeoutResponse
}

@Test
public void extractRequestTimeout(){
//#extractRequestTimeout
Duration timeout1 = Duration.create(500, TimeUnit.MILLISECONDS);
Duration timeout2 = Duration.create(1000, TimeUnit.MILLISECONDS);
Route route =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use 2 spaces as indentation for Java doc snippets as well for better readability.

path("timeout", () ->
withRequestTimeout(timeout1, () ->
extractRequestTimeout( t1 ->
withRequestTimeout(timeout2, () ->
extractRequestTimeout( t2 -> {
if (t1 == timeout1 && t2 == timeout2)
return complete(StatusCodes.OK);
else
return complete(StatusCodes.INTERNAL_SERVER_ERROR);
})
)
)
)
);
//#extractRequestTimeout
StatusCode statusCode = runRoute(system, materializer, route, "timeout").get().status();
assert (StatusCodes.OK.equals(statusCode));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,31 @@ class TimeoutDirectivesExamplesSpec extends AkkaSpec(TimeoutDirectivesInfiniteTi
runRoute(route, "timeout").status should ===(StatusCodes.EnhanceYourCalm) // the timeout response
//#withRequestTimeoutResponse
}

// read currently set timeout
"allow extraction of currently set timeout" in {
//#extractRequestTimeout
val timeout1 = 500.millis
val timeout2 = 1000.millis
val route =
path("timeout") {
withRequestTimeout(timeout1) {
extractRequestTimeout { t1 ⇒
withRequestTimeout(timeout2) {
extractRequestTimeout { t2 ⇒
complete(
if (t1 == timeout1 && t2 == timeout2) StatusCodes.OK
else StatusCodes.InternalServerError
)
}
}
}
}
}
//#extractRequestTimeout

runRoute(route, "timeout").status should ===(StatusCodes.OK)
}
}

}
Expand Down