opening files on compute servers: issue where file is blank for a moment when opened for the first time

Author: williamstein

src/packages/frontend/compute/manager.ts

@@ -42,6 +42,15 @@ export class ComputeServersManager extends EventEmitter {
     this.sync_db.close();
   };
 
+  // save the current state to the backend.  This is critical to do, e.g., before
+  // opening a file and after calling connectComputeServerToPath, since otherwise
+  // the project doesn't even know that the file should open on the compute server
+  // until after it has opened it, which is disconcerting and not efficient (but
+  // does mostly work, though it is intentionally making things really hard on ourselves).
+  save = async () => {
+    await this.sync_db.save();
+  };
+
   getComputeServers = () => {
     const servers = {};
     const cursors = this.sync_db.get_cursors().toJS();

src/packages/frontend/frame-editors/code-editor/actions.ts

@@ -258,7 +258,7 @@ export class Actions<
   // Init setting of value whenever syncstring changes -- only used in derived classes
   protected _init_syncstring_value(): void {
     this._syncstring.on("change", () => {
-      if (!this._syncstring || this._syncstring.versions().length == 0) {
+      if (!this._syncstring) {
         // edge case where actions closed but this event was still triggered, OR
         // the syncstring changed, but has not actually loaded a version from
         // disk yet, which happens with compute servers for a second.
@@ -1509,7 +1509,9 @@ export class Actions<
   syncstring_commit(): void {
     // We also skip if the syncstring hasn't yet been initialized.
     // This happens in some cases.
-    if (this._state === "closed" || this._syncstring.get_state() != "ready") return;
+    if (this._state === "closed" || this._syncstring.get_state() != "ready") {
+      return;
+    }
     if (this._syncstring != null) {
       // we pass true since here we want any UI for this or any derived
       // editor to immediately react when we commit. This is particularly
@@ -1547,7 +1549,9 @@ export class Actions<
     // note -- we don't try to set the syncstring if actions are closed
     // or the syncstring isn't initialized yet.  The latter case happens when
     // switching the file that is being edited in a frame, e.g., for latex.
-    if (this._state === "closed" || this._syncstring.get_state() != "ready") return;
+    if (this._state === "closed" || this._syncstring.get_state() != "ready") {
+      return;
+    }
     if (!do_not_exit_undo_mode) {
       // If we are NOT doing an undo operation, then setting the
       // syncstring due to any

src/packages/frontend/frame-editors/terminal-editor/connected-terminal.ts

@@ -47,6 +47,12 @@ declare const $: any;
 const SCROLLBACK = 5000;
 const MAX_HISTORY_LENGTH = 100 * SCROLLBACK;
 
+// I think this may be flaky in some cases? Not sure.  Not sure it is worth it.  I can
+// never tell the difference.  ALSO, we do so much to throttle the terminal to avoid
+// messing up the browser and/or using excessive bandwidth when people do dumb things,
+// that we can't really leverage this speed (or rather network is more of a limit).
+const ENABLE_WEBGL = false;
+
 interface Path {
   file?: string;
   directory?: string;
@@ -143,25 +149,27 @@ export class Terminal<T extends CodeEditorState = CodeEditorState> {
       throw Error("terminal.element must be defined");
     }
 
-    const webglAddon = new WebglAddon();
-    try {
-      this.terminal.loadAddon(webglAddon);
-      webglAddon.onContextLoss(() => {
-        // This really does work and properly switches back to canvas.  To convince yourself
-        // of this, open a single terminal, then open another tab with another terminal and
-        // split it about 20+ times. In the console, you'll see that the oldest webGL contexts
-        // go away. That triggers calling this function, and indeed the terminal then falls
-        // back seamlessly to canvas rendering.  Very impressive, xterm.js.
-        webglAddon.dispose();
-      });
-    } catch (err) {
-      // We have to disable the dispose when it doesn't get used, since it breaks
-      // on cleanup, and the xtermjs api has no way of removing an addon, and
-      // only catching the error on dispose later would mean leaving other things
-      // potentially not cleaned up properly.  I read the code of webglAddon.dispose
-      // and it doesn't do anything if the addon wasn't initialized.
-      webglAddon.dispose = () => {};
-      console.log(`WebGL Terminal not available (using fallback). -- ${err}`);
+    if (ENABLE_WEBGL) {
+      const webglAddon = new WebglAddon();
+      try {
+        this.terminal.loadAddon(webglAddon);
+        webglAddon.onContextLoss(() => {
+          // This really does work and properly switches back to canvas.  To convince yourself
+          // of this, open a single terminal, then open another tab with another terminal and
+          // split it about 20+ times. In the console, you'll see that the oldest webGL contexts
+          // go away. That triggers calling this function, and indeed the terminal then falls
+          // back seamlessly to canvas rendering.  Very impressive, xterm.js.
+          webglAddon.dispose();
+        });
+      } catch (err) {
+        // We have to disable the dispose when it doesn't get used, since it breaks
+        // on cleanup, and the xtermjs api has no way of removing an addon, and
+        // only catching the error on dispose later would mean leaving other things
+        // potentially not cleaned up properly.  I read the code of webglAddon.dispose
+        // and it doesn't do anything if the addon wasn't initialized.
+        webglAddon.dispose = () => {};
+        console.log(`WebGL Terminal not available (using fallback). -- ${err}`);
+      }
     }
 
     this.element = this.terminal.element;
@@ -887,6 +895,7 @@ export class Terminal<T extends CodeEditorState = CodeEditorState> {
         id,
         path: this.term_path,
       });
+      await computeServerAssociations.save();
     }
   };
 }

src/packages/frontend/project_actions.ts

@@ -1375,6 +1375,7 @@ export class ProjectActions extends Actions<ProjectStoreState> {
       id,
       path: sidePath,
     });
+    await computeServerAssociations.save();
   };
 
   //   getComputeServerIdForFile = async ({
@@ -1426,6 +1427,15 @@ export class ProjectActions extends Actions<ProjectStoreState> {
       id: selectedComputeServerId,
       path,
     });
+    // Now we save: why?
+    // Because we need to be sure the backend actually knows we want to use the compute
+    // server for the file before opening it; otherwise, it'll first get opened
+    // in the project, then later on the compute server, which is potentially VERY
+    // disconcerting and annoying, especially if the file doesn't exist.  It does
+    // work without doing this (because our design is robust to switching compute servers
+    // at any time), but it ends up with a blank file for a moment, and lots of empty files
+    // being created.
+    await computeServerAssociations.save();
     return true;
   };
 

src/packages/sync/editor/generic/sync-doc.ts

@@ -1356,6 +1356,10 @@ export class SyncDoc extends EventEmitter {
       // large or is not readable by the user. They are informed to
       // fix the problem... and once they do (and wait up to 10s),
       // this will finish.
+      //       if (!this.client.is_browser() && !this.client.is_project()) {
+      //         // FAKE DELAY!!!  Just to simulate flakiness / slow network!!!!
+      //         await delay(10000);
+      //       }
       await retry_until_success({
         f: this.init_load_from_disk.bind(this),
         max_delay: 10000,
@@ -1599,7 +1603,7 @@ export class SyncDoc extends EventEmitter {
           size = await this.load_from_disk();
           if (firstLoad) {
             dbg("emitting first-load event");
-            // this event is emittd the first time the document is ever loaded from disk.
+            // this event is emited the first time the document is ever loaded from disk.
             this.emit("first-load");
           }
           dbg("loaded");