Merge pull request #1773 from yuvipanda/async-gen

JS: Use async generator instead of callbacks + add tests

binderhub/static/js/index.js

@@ -22,7 +22,7 @@ import { BASE_URL } from "./src/constants";
 import { getBuildFormValues } from "./src/form";
 import { updateRepoText } from "./src/repo";
 
-function build(providerSpec, log, fitAddon, path, pathType) {
+async function build(providerSpec, log, fitAddon, path, pathType) {
   updateFavicon(BASE_URL + "favicon_building.ico");
   // split provider prefix off of providerSpec
   const spec = providerSpec.slice(providerSpec.indexOf("/") + 1);
@@ -52,67 +52,74 @@ function build(providerSpec, log, fitAddon, path, pathType) {
     buildToken,
   );
 
-  image.onStateChange("*", function (data) {
+  for await (const data of image.fetch()) {
+    // Write message to the log terminal if there is a message
     if (data.message !== undefined) {
       log.writeAndStore(data.message);
       fitAddon.fit();
     } else {
       console.log(data);
     }
-  });
-
-  image.onStateChange("waiting", function () {
-    $("#phase-waiting").removeClass("hidden");
-  });
-
-  image.onStateChange("building", function () {
-    $("#phase-building").removeClass("hidden");
-    log.show();
-  });
-
-  image.onStateChange("pushing", function () {
-    $("#phase-pushing").removeClass("hidden");
-  });
 
-  image.onStateChange("failed", function () {
-    $("#build-progress .progress-bar").addClass("hidden");
-    $("#phase-failed").removeClass("hidden");
-
-    $("#loader").addClass("paused");
-
-    // If we fail for any reason, show an error message and logs
-    updateFavicon(BASE_URL + "favicon_fail.ico");
-    log.show();
-    if ($("div#loader-text").length > 0) {
-      $("#loader").addClass("error");
-      $("div#loader-text p.launching").html(
-        "Error loading " + spec + "!<br /> See logs below for details.",
-      );
+    switch (data.phase) {
+      case "waiting": {
+        $("#phase-waiting").removeClass("hidden");
+        break;
+      }
+      case "building": {
+        $("#phase-building").removeClass("hidden");
+        log.show();
+        break;
+      }
+      case "pushing": {
+        $("#phase-pushing").removeClass("hidden");
+        break;
+      }
+      case "failed": {
+        $("#build-progress .progress-bar").addClass("hidden");
+        $("#phase-failed").removeClass("hidden");
+
+        $("#loader").addClass("paused");
+
+        // If we fail for any reason, show an error message and logs
+        updateFavicon(BASE_URL + "favicon_fail.ico");
+        log.show();
+        if ($("div#loader-text").length > 0) {
+          $("#loader").addClass("error");
+          $("div#loader-text p.launching").html(
+            "Error loading " + spec + "!<br /> See logs below for details.",
+          );
+        }
+        image.close();
+        break;
+      }
+      case "built": {
+        $("#phase-already-built").removeClass("hidden");
+        $("#phase-launching").removeClass("hidden");
+        updateFavicon(BASE_URL + "favicon_success.ico");
+        break;
+      }
+      case "ready": {
+        image.close();
+        // If data.url is an absolute URL, it'll be used. Else, it'll be interpreted
+        // relative to current page's URL.
+        const serverUrl = new URL(data.url, window.location.href);
+        // user server is ready, redirect to there
+        window.location.href = image.getFullRedirectURL(
+          serverUrl,
+          data.token,
+          path,
+          pathType,
+        );
+        break;
+      }
+      default: {
+        console.log("Unknown phase in response from server");
+        console.log(data);
+        break;
+      }
     }
-    image.close();
-  });
-
-  image.onStateChange("built", function () {
-    $("#phase-already-built").removeClass("hidden");
-    $("#phase-launching").removeClass("hidden");
-    updateFavicon(BASE_URL + "favicon_success.ico");
-  });
-
-  image.onStateChange("ready", function (data) {
-    image.close();
-    // If data.url is an absolute URL, it'll be used. Else, it'll be interpreted
-    // relative to current page's URL.
-    const serverUrl = new URL(data.url, window.location.href);
-    // user server is ready, redirect to there
-    window.location.href = image.getFullRedirectURL(
-      serverUrl,
-      data.token,
-      path,
-      pathType,
-    );
-  });
-
-  image.fetch();
+  }
   return image;
 }
 
@@ -170,21 +177,21 @@ function indexMain() {
     return false;
   });
 
-  $("#build-form").submit(function () {
+  $("#build-form").submit(async function (e) {
+    e.preventDefault();
     const formValues = getBuildFormValues();
     updateUrls(formValues);
-    build(
+    await build(
       formValues.providerPrefix + "/" + formValues.repo + "/" + formValues.ref,
       log,
       fitAddon,
       formValues.path,
       formValues.pathType,
     );
-    return false;
   });
 }
 
-function loadingMain(providerSpec) {
+async function loadingMain(providerSpec) {
   const [log, fitAddon] = setUpLog();
   // retrieve (encoded) filepath/urlpath from URL
   // URLSearchParams.get returns the decoded value,
@@ -205,7 +212,7 @@ function loadingMain(providerSpec) {
       }
     }
   }
-  build(providerSpec, log, fitAddon, path, pathType);
+  await build(providerSpec, log, fitAddon, path, pathType);
 
   // Looping through help text every few seconds
   const launchMessageInterval = 6 * 1000;

binderhub/templates/index.html

@@ -26,7 +26,7 @@ <h2>New to Binder? Get started with a <a href="https://the-turing-way.netlify.ap
       {% block form %}
       <form id="build-form" class="form jumbotron">
         <h4 id="form-header" class='row'>Build and launch a repository</h4>
-        <input type="hidden" id="provider_prefix" value="{{repo_providers.keys() | list | first}}"/> 
+        <input type="hidden" id="provider_prefix" value="{{repo_providers.keys() | list | first}}"/>
         <div class="form-group row">
           <label for="repository">{{(repo_providers.values() | list | first).labels.text}}</label>
           <div class="input-group">
@@ -197,6 +197,8 @@ <h3 class="text-center">How it works</h3>
 {% block footer %}
 {{ super () }}
 <script type="text/javascript">
-indexMain();
+document.addEventListener("DOMContentLoaded", async () => {
+  await indexMain();
+})
 </script>
 {% endblock footer %}

binderhub/templates/loading.html

@@ -59,6 +59,8 @@
 
 {% block footer %}
 <script type="text/javascript">
-loadingMain("{{provider_spec}}");
+document.addEventListener("DOMContentLoaded", async () => {
+  await loadingMain("{{provider_spec}}");
+})
 </script>
 {% endblock footer %}

js/packages/binderhub-client/lib/index.js

@@ -1,10 +1,11 @@
 import { NativeEventSource, EventSourcePolyfill } from "event-source-polyfill";
+import { EventIterator } from "event-iterator";
 
 // Use native browser EventSource if available, and use the polyfill if not available
 const EventSource = NativeEventSource || EventSourcePolyfill;
 
 /**
- * Build and launch a repository by talking to a BinderHub API endpoint
+ * Build (and optionally launch) a repository by talking to a BinderHub API endpoint
  */
 export class BinderRepository {
   /**
@@ -34,29 +35,54 @@ export class BinderRepository {
     if (buildToken) {
       this.buildUrl.searchParams.append("build_token", buildToken);
     }
-    this.callbacks = {};
+
+    this.eventIteratorQueue = null;
   }
 
   /**
-   * Call the BinderHub API
+   * Call the binderhub API and yield responses as they come in
+   *
+   * Returns an Async Generator yielding each item returned by the
+   * server API.
+   *
+   * @typedef Line
+   * @prop {[string]} phase The phase the build is currently in. One of: building, built, fetching, launching, ready, unknown, waiting
+   * @prop {[string]} message Human readable message to display to the user. Extra newlines must *not* be added
+   * @prop {[string]} imageName (only with built) Full name of the image that has been built
+   * @prop {[string]} binder_launch_host (only with phase=ready) The host this binderhub API request was serviced by.
+   *                                     Could be different than the host the request was made to in federated cases
+   * @prop {[string]} binder_request (only with phase=ready) Request used to construct this image, of form v2/<provider>/<repo>/<ref>
+   * @prop {[string]} binder_persistent_request (only with phase=ready) Same as binder_request, but <ref> is fully resolved
+   * @prop {[string]} binder_ref_url (only with phase=ready) A URL to the repo provider where the repo can be browsed
+   * @prop {[string]} image (only with phase=ready) Full name of the image that has been built
+   * @prop {[string]} token (only with phase=ready) Token to use to authenticate with jupyter server at url
+   * @prop {[string]} url (only with phase=ready) URL where a jupyter server has been started
+   * @prop {[string]} repo_url (only with phase=ready) URL of the repository that is ready to be launched
+   *
+   * @returns {AsyncGenerator<Line>} An async generator yielding responses from the API as they come in
    */
   fetch() {
     this.eventSource = new EventSource(this.buildUrl);
-    this.eventSource.onerror = (err) => {
-      console.error("Failed to construct event stream", err);
-      this._changeState("failed", {
-        message: "Failed to connect to event stream\n",
+    return new EventIterator((queue) => {
+      this.eventIteratorQueue = queue;
+      this.eventSource.onerror = (err) => {
+        queue.push({
+          phase: "failed",
+          message: "Failed to connect to event stream\n",
+        });
+        queue.stop();
+      };
+
+      this.eventSource.addEventListener("message", (event) => {
+        // console.log("message received")
+        // console.log(event)
+        const data = JSON.parse(event.data);
+        // FIXME: fix case of phase/state upstream
+        if (data.phase) {
+          data.phase = data.phase.toLowerCase();
+        }
+        queue.push(data);
       });
-    };
-    this.eventSource.addEventListener("message", (event) => {
-      const data = JSON.parse(event.data);
-      // FIXME: Rename 'phase' to 'state' upstream
-      // FIXME: fix case of phase/state upstream
-      let state = null;
-      if (data.phase) {
-        state = data.phase.toLowerCase();
-      }
-      this._changeState(state, data);
     });
   }
 
@@ -67,6 +93,10 @@ export class BinderRepository {
     if (this.eventSource !== undefined) {
       this.eventSource.close();
     }
+    if (this.eventIteratorQueue !== null) {
+      // Stop any currently running fetch() iterations
+      this.eventIteratorQueue.stop();
+    }
   }
 
   /**
@@ -113,29 +143,4 @@ export class BinderRepository {
     url.searchParams.append("token", token);
     return url;
   }
-
-  /**
-   * Add callback whenever state of the current build changes
-   *
-   * @param {str} state The state to add this callback to. '*' to add callback for all state changes
-   * @param {*} cb Callback function to call whenever this state is reached
-   */
-  onStateChange(state, cb) {
-    if (this.callbacks[state] === undefined) {
-      this.callbacks[state] = [cb];
-    } else {
-      this.callbacks[state].push(cb);
-    }
-  }
-
-  _changeState(state, data) {
-    [state, "*"].map((key) => {
-      const callbacks = this.callbacks[key];
-      if (callbacks) {
-        for (let i = 0; i < callbacks.length; i++) {
-          callbacks[i](data);
-        }
-      }
-    });
-  }
 }

js/packages/binderhub-client/package.json

@@ -14,6 +14,7 @@
   },
   "homepage": "https://github.com/jupyterhub/binderhub#readme",
   "dependencies": {
-    "event-source-polyfill": "^1.0.31"
+    "event-source-polyfill": "^1.0.31",
+    "event-iterator": "^2.0.0"
   }
 }