fix router stuff

index.js

@@ -24,9 +24,17 @@ const wsRouter = express.Router()
 let viewpath = path.join(__dirname, "views");
 let eta = new Eta({ views: viewpath, cache: false, autoEscape: true });
 
-let routes = { GET: [], POST: [], PUT: [], DELETE: [] };
+// A complete, immutable snapshot of the routes currently available to the
-const wsRoutes = {}
+// runtime. Route-changing writes replace this object only after the new index
-const wsConnections = {}
+// has been built, so requests never observe a partially refreshed table.
+let routeIndex = Object.freeze({
+  GET: Object.freeze([]),
+  POST: Object.freeze([]),
+  PUT: Object.freeze([]),
+  DELETE: Object.freeze([]),
+  WS: Object.freeze([]),
+});
+const wsConnections = new Map();
 
 const INSPECT_OPTS = { showHidden: false, depth: null, colors: false, compact: false };
 function inspectArgs(args) {
@@ -154,84 +162,110 @@ function routeWithPrefix(route) {
 }
 
 
-function bootstrapWebsocketHandler(route) {
+function compileWebsocketHandler(route) {
-  // todo only expose app when running the handler, should not be available to the handler itself
-  // need to move to runscript or whatever...or remove app because we can just call it using the handler?
   try {
+    // Keep the existing WS handler context for compatibility. Restricting the
+    // exposed capabilities is a separate runtime-sandboxing change.
     const context = bootstrapContext(route.structure_id, route.id, { app });
-    let handler = vm.runInContext(`${route.handler}\n\nhandler;`, context);
+    const handler = vm.runInContext(`${route.handler}\n\nhandler;`, context);
-
-    if (!wsRoutes[routeWithPrefix(route)]) {
-      wsConnections[routeWithPrefix(route)] = new Set();
-      // initialize ws router. future updates will only require updating
-      // the wsRoutes dict, not create a whole new route
-      wsRouter.ws(routeWithPrefix(route), (ws, req) => {
-        const myOn = ws.on.bind(ws)
-        ws.on = (thing, cb) => {
-          myOn(thing, (...args) => {
-            try {
-              cb(...args)
-            }
-            catch (e) {
-              logError(route.structure_id, route.id, e);
-            }
-          })
-        }
-        ws.render = (template, context = {}) => {
-          const structure = model.getStructure(route.structure_id);
-          context.route = makeRoute(structure);
-          ws.send(
-            decorate(renderTemplate(route.structure_id, template, context), {
-              headInjection: structure.head_injection,
-              source: sourceFor(route, req),
-              fragment: true,
-            }),
-          );
-        };
-
-        ws.clients = wsConnections[routeWithPrefix(route)];
-
-        return wsRoutes[routeWithPrefix(route)](ws, req)
-      });
-    }
-
-    wsRoutes[routeWithPrefix(route)] = (ws, req) => {
-      try {
-        return handler(ws, req)
-      } catch (e) {
-         logError(route.structure_id, route.id, e);
-      }
-    }
     model.updateRoute({ ...route, error: null });
+    return handler;
   } catch (e) {
     logError(route.structure_id, route.id, e);
     model.updateRoute({ ...route, error: e.stack });
+    return null;
   }
 }
 
 function buildRoutes() {
-  let newRoutes = {
+  const nextIndex = {
     GET: [],
     POST: [],
     PUT: [],
     DELETE: [],
+    WS: [],
   };
 
   for (let route of model.getAllRoutes()) {
     const p = routeWithPrefix(route);
-    if (route.verb == "WS") {
+    const entry = {
-      bootstrapWebsocketHandler(route);
+      matcher: match(p, { decode: decodeURIComponent }),
-    } else {
+      route: Object.freeze({ ...route }),
-      newRoutes[route.verb].push({
+    };
-        matcher: match(p, { decode: decodeURIComponent }),
+    if (route.verb === "WS") entry.handler = compileWebsocketHandler(route);
-        id: route.id,
+    nextIndex[route.verb].push(Object.freeze(entry));
-        path: route.path,
-      });
-    }
   }
-  routes = newRoutes;
+
+  for (const verb of Object.keys(nextIndex)) Object.freeze(nextIndex[verb]);
+  routeIndex = Object.freeze(nextIndex);
 }
 
+function findRuntimeRoute(verb, requestPath) {
+  for (const entry of routeIndex[verb] || []) {
+    const result = entry.matcher(requestPath);
+    if (result) return { ...entry, params: result.params };
+  }
+  return null;
+}
+
+function websocketRequestPath(req) {
+  // express-ws internally appends `/.websocket` before routing the upgrade.
+  return req.path.replace(/\/?\.websocket$/, "") || "/";
+}
+
+// One permanent WebSocket endpoint dispatches through the same replaceable
+// route index as HTTP. Moving or deleting a DB-backed WS route therefore does
+// not leave an old Express route registered forever.
+wsRouter.ws("*", (ws, req) => {
+  const found = findRuntimeRoute("WS", websocketRequestPath(req));
+  if (!found || !found.handler) {
+    return ws.close(1008, "WebSocket route not found");
+  }
+
+  const { route } = found;
+  req.params = found.params;
+  let clients = wsConnections.get(route.id);
+  if (!clients) {
+    clients = new Set();
+    wsConnections.set(route.id, clients);
+  }
+  clients.add(ws);
+  ws.once("close", () => {
+    clients.delete(ws);
+    if (clients.size === 0) wsConnections.delete(route.id);
+  });
+
+  const originalOn = ws.on.bind(ws);
+  ws.on = (event, callback) =>
+    originalOn(event, (...args) => {
+      try {
+        callback(...args);
+      } catch (e) {
+        logError(route.structure_id, route.id, e);
+      }
+    });
+
+  ws.clients = clients;
+  ws.render = (template, context = {}) => {
+    const structure = model.getStructure(route.structure_id);
+    context.route = makeRoute(structure);
+    ws.send(
+      decorate(renderTemplate(route.structure_id, template, context), {
+        headInjection: structure.head_injection,
+        source: sourceFor(route, req),
+        fragment: true,
+      }),
+    );
+  };
+
+  try {
+    return found.handler(ws, req);
+  } catch (e) {
+    logError(route.structure_id, route.id, e);
+    return ws.close(1011, "WebSocket handler failed");
+  }
+});
+
 model.applyMigrations();
 buildRoutes();
 
@@ -531,9 +565,7 @@ app.put("/workshop/:structure_id/route/:route_id", (req, res) => {
     const latestPage = model.getLatestScaffoldPage(req.params.route_id)
     model.updateScaffoldPage({ ...latestPage, content: req.body.scaffold_page })
   }
-  if (route.verb == "WS") {
+  buildRoutes();
-    bootstrapWebsocketHandler({ ...route, ...req.body })
-  }
   return res.send("good");
 });
 
@@ -764,6 +796,7 @@ app.put("/workshop/:structure_id/settings", (req, res) => {
     route_prefix: routePrefix,
     head_injection: req.body.head_injection,
   });
+  buildRoutes();
   struct = model.getStructure(structId);
 
   res.send("success!");
@@ -807,21 +840,11 @@ app.all("*", async (req, res) => {
   const verb = req.method;
 
   try {
-    let routeMatch = null;
+    const routeMatch = findRuntimeRoute(verb, req_path);
-
-    // routes[verb] is ordered most-recently-updated first (getAllRoutes sorts
-    // by updated_at DESC), so the first match is the most recently saved route.
-    for (let { matcher, id } of routes[verb]) {
-      let matchFromRoute = matcher(req_path);
-      if (matchFromRoute) {
-        routeMatch = { params: matchFromRoute.params, id: id };
-        break;
-      }
-    }
 
     if (routeMatch) {
       req.params = routeMatch.params;
-      const route = model.getRoute(routeMatch.id);
+      const route = routeMatch.route;
 
       const structure = model.getStructure(route.structure_id);