Merge branch 'master' into fs2

Author: williamstein

src/packages/conat/core/client.ts

@@ -510,7 +510,10 @@ export class Client extends EventEmitter {
       reconnection: true,
     });
 
-    this.conn.on("info", (info) => {
+    this.conn.on("info", (info, ack) => {
+      if (typeof ack == "function") {
+        ack();
+      }
       const firstTime = this.info == null;
       this.info = info;
       this.emit("info", info);

src/packages/conat/core/server.ts

@@ -53,7 +53,7 @@ import {
 import { Patterns } from "./patterns";
 import { is_array } from "@cocalc/util/misc";
 import { UsageMonitor } from "@cocalc/conat/monitor/usage";
-import { once } from "@cocalc/util/async-utils";
+import { once, until } from "@cocalc/util/async-utils";
 import {
   clusterLink,
   type ClusterLink,
@@ -940,7 +940,7 @@ export class ConatServer extends EventEmitter {
       this.subscriptions[id] = new Set<string>();
     }
 
-    socket.emit("info", { ...this.info(), user });
+    this.sendInfo(socket, user);
 
     socket.on("stats", ({ recv0 }) => {
       const s = this.stats[socket.id];
@@ -1086,6 +1086,38 @@ export class ConatServer extends EventEmitter {
     });
   };
 
+  sendInfo = async (socket, user) => {
+    // we send info with an ack because I think sometimes the initial "info"
+    // message gets dropped, leaving a broken hanging connection that never
+    // does anything (until the user explicitly refreshes their browser).
+    // I did see what is probably this in production frequently.
+    try {
+      await until(
+        async () => {
+          if (!socket.conn?.readyState.startsWith("o")) {
+            // logger.debug(`failed to send "info" message to ${socket.id}`);
+            // readyState not defined or not opened or opening, so connection must
+            // have been closed before success.
+            return true;
+          }
+          try {
+            await socket
+              .timeout(7500)
+              .emitWithAck("info", { ...this.info(), user });
+            return true;
+          } catch (err) {
+            // logger.debug(`error sending "info" message to ${socket.id}`, err);
+            return false;
+          }
+        },
+        { min: 5000, max: 30000, timeout: 120_000 },
+      );
+    } catch {
+      // never ack'd "info" after a few minutes -- could just be an old client,
+      // so don't do anything at this point.
+    }
+  };
+
   address = () => getServerAddress(this.options);
 
   // create new client in the same process connected to this server.
@@ -1349,8 +1381,8 @@ export class ConatServer extends EventEmitter {
     const usage = async () => {
       return { [this.id]: this.usage.stats() };
     };
-    // user has to explicitly refresh there browser after
-    // being disconnected this way
+    // user has to explicitly refresh their browser after
+    // being disconnected this way:
     const disconnect = async (ids: string | string[]) => {
       if (typeof ids == "string") {
         ids = [ids];

src/packages/conat/service/service.ts

@@ -210,7 +210,12 @@ export class ConatService extends EventEmitter {
         await mesg.respond(resp);
       } catch (err) {
         const data = { error: `${err}` };
-        await mesg.respond(data);
+        try {
+          await mesg.respond(data);
+        } catch (err2) {
+          // do not crash on sending an error report:
+          logger.debug("WARNING: unable to send error", this.name, err, err2);
+        }
       }
     }
   };

src/packages/conat/socket/util.ts

@@ -13,7 +13,7 @@ export type Role = "client" | "server";
 // socketio and use those to manage things.  This ping
 // is entirely a "just in case" backup if some event
 // were missed (e.g., a kill -9'd process...)
-export const PING_PONG_INTERVAL = 60000;
+export const PING_PONG_INTERVAL = 90000;
 
 // We queue up unsent writes, but only up to a point (to not have a huge memory issue).
 // Any write beyond this size result in an exception.
@@ -24,8 +24,8 @@ export const PING_PONG_INTERVAL = 60000;
 export const DEFAULT_MAX_QUEUE_SIZE = 1000;
 
 export let DEFAULT_COMMAND_TIMEOUT = 10_000;
-export let DEFAULT_KEEP_ALIVE = 90_000;
-export let DEFAULT_KEEP_ALIVE_TIMEOUT = 15_000;
+export let DEFAULT_KEEP_ALIVE = 25_000;
+export let DEFAULT_KEEP_ALIVE_TIMEOUT = 10_000;
 
 export function setDefaultSocketTimeouts({
   command = DEFAULT_COMMAND_TIMEOUT,

src/packages/frontend/client/idle.ts

@@ -84,7 +84,7 @@ export class IdleClient {
 
     if (NEVER_TIMEOUT_VISIBLE) {
       // If the document is visible right now, then we
-      // reset the idle timeout., just as if the mouse moved.  This means
+      // reset the idle timeout, just as if the mouse moved.  This means
       // that users never get the standby timeout if their current browser
       // tab is considered visible according to the Page Visibility API
       // https://developer.mozilla.org/en-US/docs/Web/API/Page_Visibility_API
@@ -110,6 +110,7 @@ export class IdleClient {
       // so that if the user sees the idle banner and immediately
       // dismisses it, then the experience is less disruptive.
       this.delayed_disconnect = setTimeout(() => {
+        this.delayed_disconnect = undefined;
         console.log("Entering standby mode");
         this.standbyMode = true;
         // console.log("idle timeout: disconnect!");

src/packages/hub/proxy/file-download.ts

@@ -0,0 +1,71 @@
+import { readFile as readProjectFile } from "@cocalc/conat/files/read";
+import { once } from "events";
+import { path_split } from "@cocalc/util/misc";
+import mime from "mime-types";
+import getLogger from "../logger";
+
+const DANGEROUS_CONTENT_TYPE = new Set(["image/svg+xml" /*, "text/html"*/]);
+
+const logger = getLogger("hub:proxy:file-download");
+
+// assumes request has already been authenticated!
+
+export async function handleFileDownload(req, res, url, project_id) {
+  logger.debug("handling the request via conat file streaming", url);
+  const i = url.indexOf("files/");
+  const compute_server_id = req.query.id ?? 0;
+  let j = url.lastIndexOf("?");
+  if (j == -1) {
+    j = url.length;
+  }
+  const path = decodeURIComponent(url.slice(i + "files/".length, j));
+  logger.debug("conat: get file", { project_id, path, compute_server_id, url });
+  const fileName = path_split(path).tail;
+  const contentType = mime.lookup(fileName);
+  if (req.query.download != null || DANGEROUS_CONTENT_TYPE.has(contentType)) {
+    const fileNameEncoded = encodeURIComponent(fileName)
+      .replace(/['()]/g, escape)
+      .replace(/\*/g, "%2A");
+    res.setHeader(
+      "Content-disposition",
+      `attachment; filename*=UTF-8''${fileNameEncoded}`,
+    );
+  }
+  res.setHeader("Content-type", contentType);
+
+  let headersSent = false;
+  res.on("finish", () => {
+    headersSent = true;
+  });
+  try {
+    for await (const chunk of await readProjectFile({
+      project_id,
+      compute_server_id,
+      path,
+      // allow a long download time (1 hour), since files can be large and
+      // networks can be slow.
+      maxWait: 1000 * 60 * 60,
+    })) {
+      if (res.writableEnded || res.destroyed) {
+        break;
+      }
+      if (!res.write(chunk)) {
+        await once(res, "drain");
+      }
+    }
+    res.end();
+  } catch (err) {
+    logger.debug(
+      "ERROR streaming file",
+      { project_id, compute_server_id, path },
+      err,
+    );
+    if (!headersSent) {
+      res.statusCode = 500;
+      res.end("Error reading file.");
+    } else {
+      // Data sent, forcibly kill the connection
+      res.destroy(err);
+    }
+  }
+}

src/packages/hub/proxy/handle-request.ts

@@ -10,13 +10,8 @@ import { stripBasePath } from "./util";
 import { ProjectControlFunction } from "@cocalc/server/projects/control";
 import siteUrl from "@cocalc/database/settings/site-url";
 import { parseReq } from "./parse";
-import { readFile as readProjectFile } from "@cocalc/conat/files/read";
-import { path_split } from "@cocalc/util/misc";
-import { once } from "@cocalc/util/async-utils";
 import hasAccess from "./check-for-access-to-project";
-import mime from "mime-types";
-
-const DANGEROUS_CONTENT_TYPE = new Set(["image/svg+xml" /*, "text/html"*/]);
+import { handleFileDownload } from "./file-download";
 
 const logger = getLogger("proxy:handle-request");
 
@@ -84,7 +79,6 @@ export default function init({ projectControl, isPersonal }: Options) {
     // TODO: parseReq is called again in getTarget so need to refactor...
     const { type, project_id } = parsed;
     if (type == "files") {
-      dbg("handling the request via conat file streaming");
       if (
         !(await hasAccess({
           project_id,
@@ -96,43 +90,7 @@ export default function init({ projectControl, isPersonal }: Options) {
       ) {
         throw Error(`user does not have read access to project`);
       }
-      const i = url.indexOf("files/");
-      const compute_server_id = req.query.id ?? 0;
-      let j = url.lastIndexOf("?");
-      if (j == -1) {
-        j = url.length;
-      }
-      const path = decodeURIComponent(url.slice(i + "files/".length, j));
-      dbg("conat: get file", { project_id, path, compute_server_id, url });
-      const fileName = path_split(path).tail;
-      const contentType = mime.lookup(fileName);
-      if (
-        req.query.download != null ||
-        DANGEROUS_CONTENT_TYPE.has(contentType)
-      ) {
-        const fileNameEncoded = encodeURIComponent(fileName)
-          .replace(/['()]/g, escape)
-          .replace(/\*/g, "%2A");
-        res.setHeader(
-          "Content-disposition",
-          `attachment; filename*=UTF-8''${fileNameEncoded}`,
-        );
-      }
-      res.setHeader("Content-type", contentType);
-      for await (const chunk of await readProjectFile({
-        project_id,
-        compute_server_id,
-        path,
-        // allow a long download time (1 hour), since files can be large and
-        // networks can be slow.
-        maxWait: 1000 * 60 * 60,
-      })) {
-        if (!res.write(chunk)) {
-          // backpressure -- wait for it to resolve
-          await once(res, "drain");
-        }
-      }
-      res.end();
+      await handleFileDownload(req, res, url, project_id);
       return;
     }
 
@@ -182,8 +140,14 @@ export default function init({ projectControl, isPersonal }: Options) {
       await handleProxyRequest(req, res);
     } catch (err) {
       const msg = `WARNING: error proxying request ${req.url} -- ${err}`;
-      res.writeHead(426, { "Content-Type": "text/html" });
-      res.end(msg);
+      try {
+        // this will fail if handleProxyRequest already wrote a header, so we
+        // try/catch it.
+        res.writeHead(500, { "Content-Type": "text/html" });
+      } catch {}
+      try {
+        res.end(msg);
+      } catch {}
       // Not something to log as an error -- just debug; it's normal for it to happen, e.g., when
       // a project isn't running.
       logger.debug(msg);

src/packages/hub/servers/express-app.ts

@@ -2,7 +2,6 @@
 The main hub express app.
 */
 
-import compression from "compression";
 import cookieParser from "cookie-parser";
 import express from "express";
 import ms from "ms";
@@ -33,6 +32,9 @@ import basePath from "@cocalc/backend/base-path";
 import { initConatServer } from "@cocalc/server/conat/socketio";
 import { conatSocketioCount } from "@cocalc/backend/data";
 
+// NOTE: we are not using compression because that interferes with streaming file download,
+// and could be generally confusing.
+
 // Used for longterm caching of files. This should be in units of seconds.
 const MAX_AGE = Math.round(ms("10 days") / 1000);
 const SHORT_AGE = Math.round(ms("10 seconds") / 1000);
@@ -80,12 +82,6 @@ export default async function init(opts: Options): Promise<{
     app.use(vhostShare());
   }
 
-  // Enable compression, as suggested by
-  //   http://expressjs.com/en/advanced/best-practice-performance.html#use-gzip-compression
-  // NOTE "Express runs everything in order" --
-  // https://github.com/expressjs/compression/issues/35#issuecomment-77076170
-  app.use(compression());
-
   app.use(cookieParser());
 
   // Install custom middleware to track response time metrics via prometheus

src/packages/util/smc-version.js

@@ -1,2 +1,2 @@
 /* autogenerated by the update_version script */
-exports.version=1752527312;
+exports.version=1752795879;