Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Reprocess failed ajax request. #1366

Closed
wants to merge 2 commits into from

4 participants

@jeppenejsum
Owner

When an ajax request fails the client will resend the request.

With this patch, if the failure is caused by the server (http code >500) the retry-request will actually be processed again instead of being satisfied by the previously computed response (which was a failure)

@jeppenejsum jeppenejsum Reprocess failed ajax request.
When an ajax request fails the client will resend the request.

With this patch, if the failure is caused by the server (http code >500) the retry-request will actually be processed again instead of being satisfied by the previously computed response (which was a failure)
de1328c
...kit/src/main/scala/net/liftweb/http/LiftServlet.scala
@@ -619,8 +631,20 @@ class LiftServlet extends Loggable {
val ret:Box[LiftResponse] =
nextAction match {
case Left(future) =>
- val result = runAjax(liftSession, requestState)
- future.satisfy(result)
+ val result = runAjax(liftSession, requestState) map CachedResponse
+
+ result foreach {resp =>
+ if (resp.failed_?) {
@Shadowfiend Owner

Given that this is only used here, I'm not sure having an extra indirection through the CachedResponse class is needed. Why not just perform the code check directly here and act accordingly? Something like:

result match {
  case Full(response) if response.code >= 500 && response.code < 600 =>
    // The request failed...
    liftSession.with...
  case other =>
    future.satisfy(other)
}
@Shadowfiend Owner

Oh, in this case, for any pending requests, we should also still satisfy the future with the result.

@jeppenejsum Owner

But LiftResponse doesnt have code, only toResponse. So we end up calling toResponse twice (here and where the response is actually sent)

I'm unsure why this is so but assumes there's a reason :-)

@Shadowfiend Owner

Aha, I see. That does indeed complicate things. @dpp does this look reasonable to you then?

@dpp Owner
dpp added a note

Please give it a try and see how it works in the wild. ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jeppenejsum jeppenejsum was assigned
@fmpwizard
Owner

As discussed on the mailing list, closing this pull request until Jeppe (or anyone else) has time to implement "Oh, in this case, for any pending requests, we should also still satisfy the future with the result." (Just to avoid merging it to master until it is ready)

@fmpwizard fmpwizard closed this
@jeppenejsum
Owner

Updated. Sorry for the delay :-)

@jeppenejsum jeppenejsum reopened this
@Shadowfiend
Owner

+1 looks good.

@fmpwizard
Owner

rebased to master

@fmpwizard fmpwizard closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 22, 2012
  1. @jeppenejsum

    Reprocess failed ajax request.

    jeppenejsum authored
    When an ajax request fails the client will resend the request.
    
    With this patch, if the failure is caused by the server (http code >500) the retry-request will actually be processed again instead of being satisfied by the previously computed response (which was a failure)
Commits on Dec 3, 2012
  1. @jeppenejsum
This page is out of date. Refresh to see the latest.
Showing with 24 additions and 2 deletions.
  1. +24 −2 web/webkit/src/main/scala/net/liftweb/http/LiftServlet.scala
View
26 web/webkit/src/main/scala/net/liftweb/http/LiftServlet.scala
@@ -28,6 +28,18 @@ import auth._
import provider._
import json.JsonAST.JValue
+/**
+ * Wrap a LiftResponse and cache the result to avoid computing the actual response
+ * more than once
+ */
+private [http] case class CachedResponse(wrapped: LiftResponse) extends LiftResponse {
+ private val _cachedResponse = wrapped.toResponse
+
+ def toResponse = _cachedResponse
+
+ // Should we retry request processing
+ def failed_? = _cachedResponse.code >= 500 && _cachedResponse.code < 600
+}
class LiftServlet extends Loggable {
private var servletContext: HTTPContext = null
@@ -619,9 +631,19 @@ class LiftServlet extends Loggable {
val ret:Box[LiftResponse] =
nextAction match {
case Left(future) =>
- val result = runAjax(liftSession, requestState)
- future.satisfy(result)
+ val result = runAjax(liftSession, requestState) map CachedResponse
+
+ if (result.exists(_.failed_?)) {
+ // The request failed. The client will retry it, so
+ // remove it from the list of current Ajax requests that
+ // needs to be satisfied so we re-process the next request
+ // from scratch
+ liftSession.withAjaxRequests { currentAjaxRequests =>
+ currentAjaxRequests.remove(RenderVersion.get)
+ }
+ }
+ future.satisfy(result)
result
case Right(future) =>
Something went wrong with that request. Please try again.