Comet Rehydration #1770

joescii · 2016-02-14T02:53:56Z

As discussed on this ML thread, comet rehydration is another step towards supporting Lift server instance failover.

This feature is DISABLED by default. It can be enabled with these two lines in Boot:

LiftRules.redirectAsyncOnSessionLoss = false
LiftRules.noCometSessionCmd.default.set(() => JsCmds.Run("lift.rehydrateComets()"))

Rehydrating comets works by refetching the html from the Lift server. The html is slapped into an iframe and scraped for the comet GUID and version pairs. The toWatch is replaced with the new comet pairs effectively allowing the newly created comets to take over.

As with the LiftRules.putAjaxFnsInContainerSession PR, this feature can be exercised with this project but using the /comet page.

…hydration

…g the toWatch directly from the HTML

…ence the browser will not attempt to run any scripts

…tVanilla

joescii · 2016-02-14T03:26:26Z

I vote for marking codacy's complaint about cyclomatic complexity as WONTFIX.

farmdawgnation · 2017-05-28T21:26:22Z

Hey @joescii - what are your thoughts on the stability of this? Is this something that could potentially be included in 3.1 or nah?

Shadowfiend · 2017-06-12T14:00:05Z

Going to drop the milestone here. We can restore it when timelines are a little clearer, I think.

farmdawgnation · 2017-09-13T02:40:19Z

@joescii I see you've been periodically merging master here. LMK When you think it's ready for review/ testing

joescii · 2017-10-03T18:28:45Z

web/webkit/src/main/resources/toserve/lift.js

+      if (!dataType) {
+        dataType = "script";
+      }
+


@Shadowfiend This is the change I was talking about in this ML thread https://groups.google.com/forum/#!topic/liftweb/KdH4kgZ-5ug

Because Javascript, this change is backward compatible. I don't think we have to worry too much about this change so long as the dataType check works as we expect.

farmdawgnation

Have some changes to request, but it looks promising!

farmdawgnation · 2017-11-04T14:35:34Z

web/webkit/src/main/resources/toserve/lift.js

+        iframe.appendChild(doc);
+        doc.innerHTML = html;
+
+        toWatch = {};


Is this attached to window or a lift object on the window?

It's declared up at the top at line 20

farmdawgnation · 2017-11-04T14:37:23Z

web/webkit/src/main/resources/toserve/lift.js

+        restartComet();
+
+        // Under the impression that a parentless node needn't be deleted.
+        // It should be a candidate for GC after it goes out of scope


I think this is right. We should be able to remove this comment.

farmdawgnation · 2017-11-04T14:37:47Z

web/webkit/src/main/resources/toserve/lift.js

+      }
+      function onFailure(err) {
+        // Try again??
+        settings.logError(err);


Could we fall back to the classic comet refresh method? (Reloading the page?)

Do you think we should hard code a refresh here, or make it a LiftRules setting?

farmdawgnation · 2017-11-04T14:39:24Z

web/webkit/src/main/scala/net/liftweb/builtin/snippet/Comet.scala

-    cometActor.parentTag.copy(child = cometHtml) % ("id" -> containerId)
+  private [this] def buildContainer(cometHtml: NodeSeq, cometActor: LiftCometActor, containerId: String): NodeSeq = {
+    val cometVersion = http.S.requestCometVersions.is
+      .collectFirst { case CometVersionPair(guid, ver) if guid == cometActor.uniqueId => ver }


Please break this into multiple lines for readability.

farmdawgnation · 2017-11-04T14:39:53Z

web/webkit/src/main/scala/net/liftweb/builtin/snippet/Comet.scala

+      .collectFirst { case CometVersionPair(guid, ver) if guid == cometActor.uniqueId => ver }
+      .getOrElse("")
+
+    cometActor.parentTag.copy(child = cometHtml) % ("id" -> containerId) % ("data-lift-comet-version" -> cometVersion)


Please break this into multiple lines for readability.

farmdawgnation · 2017-11-04T14:45:12Z

web/webkit/src/main/resources/toserve/lift.js

+        var iframe = document.createElement("iframe");
+        var doc = document.createElement("html");
+        iframe.appendChild(doc);
+        doc.innerHTML = html;


Just a reminder to be very careful about where the html value in this method comes from. My understanding is that our risk here is pretty minor as doc andiframe are never attached to the page, but whenever I see it in a PR I gotta mention it.

(Old habits die hard.)

Yeah, this is fundamental to the design of this feature. Essentially we reload the page from Lift, and steal all of the comet IDs from the resulting HTML.

farmdawgnation · 2017-11-04T14:46:18Z

web/webkit/src/main/resources/toserve/lift.js

+      if (!dataType) {
+        dataType = "script";
+      }
+


Because Javascript, this change is backward compatible. I don't think we have to worry too much about this change so long as the dataType check works as we expect.

farmdawgnation

I took this out for a spin and it worked like a charm!

How would you feel about providing a helper in LiftRules that makes the correct changes to the configuration? Something like...

LiftRules.enableCometRehydration()

What do you think?

Side note: I'd love to ship this next month as a part of 3.2.0-RC1 if you feel comfortable with it.

Shadowfiend · 2017-11-24T19:39:48Z

Side note

❗️

joescii · 2017-11-29T19:58:59Z

@farmdawgnation I dig that idea. So basically something like

def enableCometRehydration(): Unit = {
  LiftRules.redirectAsyncOnSessionLoss = false
  LiftRules.noCometSessionCmd.default.set(() => JsCmds.Run("lift.rehydrateComets()"))
}

farmdawgnation · 2017-11-29T20:45:31Z

Yup!

farmdawgnation · 2017-12-06T01:39:48Z

@joescii Bump! We have about 10 days to get this in for the RC! :)

joescii · 2017-12-06T21:13:23Z

web/webkit/src/main/scala/net/liftweb/http/LiftRules.scala

+    LiftRules.redirectAsyncOnSessionLoss = false
+    LiftRules.noCometSessionCmd.default.set(() => JsCmds.Run("lift.rehydrateComets()"))
+  }
+


@farmdawgnation @Shadowfiend I feel like the hardest part to get right here is the comment...

I think that the comment strikes the right chord overall. The one guidance we might add would be to say that you should code applications depending on this behavior to be able to reconstruct comet state from cookies or from information available from the client (e.g. JS state, state stores, etc)

If you feel like that would be a good addition I say add it. Otherwise, I'm going to approve this PR.

Comment updated

farmdawgnation

farmdawgnation · 2017-12-14T20:39:16Z

joescii added 12 commits February 6, 2016 15:32

Bump to 3.1-SNAPSHOT

b480f05

Adding data-lift-comet to comet outer tags so we can grab them for re…

a0b9ddb

…hydration

First attempt at rehydrateComets() that requires jquery

fa84b7c

Adding the comet version to the comet divs to pave the way to scrapin…

b264516

…g the toWatch directly from the HTML

Converting a find/map to collectFirst

c7cc955

Building up toWatch by scraping the HTML

b3806b4

A jquery-free implementation which will not run the page's javascript

5668778

Replacing the old comet divs with the new comet divs

5410939

Putting the comet rehydration page into an iframe to have more confid…

9610be1

…ence the browser will not attempt to run any scripts

getComets() and rehydrateComets() take advantage of liftJQuery vs lif…

ceb089d

…tVanilla

Using document.querySelectorAll when available in liftVanilla.getComets

3801b69

Handling some good finds by jshint

bf3048d

joescii added this to the 3.1 milestone Feb 14, 2016

Shadowfiend removed this from the 3.1.0-RC1 milestone Jun 12, 2017

Joe Barnes added 4 commits June 19, 2017 09:06

Merge branch 'master' into joe-comet-rehydrate

0bd4ac0

Merge branch 'master' into joe-comet-rehydrate

979aafa

Merge branch 'master' into joe-comet-rehydrate

b834918

Merge branch 'master' into joe-comet-rehydrate

cbe0a7c

Changing comet rehydration to use ajaxGet instead of ajaxPost

4c1e2ad

joescii changed the title ~~[WIP] Comet Rehydration~~ Comet Rehydration Oct 3, 2017

joescii commented Oct 3, 2017

View reviewed changes

farmdawgnation requested changes Nov 4, 2017

View reviewed changes

Joe Barnes added 2 commits November 14, 2017 14:18

Removing comment after code review with farmdawg

62cb1ea

Breaking up long lines of code after code review with farmdawg

1cc64e1

farmdawgnation reviewed Nov 24, 2017

View reviewed changes

Defined LiftRules.enableCometRehydration

6416673

joescii commented Dec 6, 2017

View reviewed changes

farmdawgnation approved these changes Dec 10, 2017

View reviewed changes

Updating enableCometRehydration's scaladoc per code review

76ee394

farmdawgnation approved these changes Dec 11, 2017

View reviewed changes

farmdawgnation merged commit ba9c893 into master Dec 14, 2017

farmdawgnation deleted the joe-comet-rehydrate branch December 14, 2017 20:32

farmdawgnation added this to the 3.2.0-RC1 milestone Dec 14, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comet Rehydration #1770

Comet Rehydration #1770

joescii commented Feb 14, 2016

joescii commented Feb 14, 2016

farmdawgnation commented May 28, 2017

Shadowfiend commented Jun 12, 2017

farmdawgnation commented Sep 13, 2017

joescii Oct 3, 2017

farmdawgnation Nov 4, 2017

farmdawgnation left a comment

farmdawgnation Nov 4, 2017

joescii Nov 14, 2017

farmdawgnation Nov 4, 2017

farmdawgnation Nov 4, 2017

joescii Nov 14, 2017

farmdawgnation Nov 4, 2017

farmdawgnation Nov 4, 2017

farmdawgnation Nov 4, 2017

joescii Nov 14, 2017

farmdawgnation Nov 4, 2017

farmdawgnation left a comment

Shadowfiend commented Nov 24, 2017

joescii commented Nov 29, 2017

farmdawgnation commented Nov 29, 2017

farmdawgnation commented Dec 6, 2017

joescii Dec 6, 2017

farmdawgnation Dec 10, 2017

joescii Dec 11, 2017

farmdawgnation left a comment

farmdawgnation commented Dec 14, 2017

Comet Rehydration #1770

Comet Rehydration #1770

Conversation

joescii commented Feb 14, 2016

joescii commented Feb 14, 2016

farmdawgnation commented May 28, 2017

Shadowfiend commented Jun 12, 2017

farmdawgnation commented Sep 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

farmdawgnation left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

farmdawgnation left a comment

Choose a reason for hiding this comment

Shadowfiend commented Nov 24, 2017

joescii commented Nov 29, 2017

farmdawgnation commented Nov 29, 2017

farmdawgnation commented Dec 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

farmdawgnation left a comment

Choose a reason for hiding this comment

farmdawgnation commented Dec 14, 2017