fix #7066 -- deal with updating nextjs completely breaking our use of…

… websockets.

- this fix took HOURS of reading the nextjs source code to figure out,
  but it's a good fix, and I improved our docs/comments a lot
- I have to say... the nextjs source code is quite impressive in terms
  of quality!

src/packages/hub/hub.ts

@@ -221,13 +221,12 @@ async function startServer(): Promise<void> {
     nextServer: !!program.nextServer,
     cert: program.httpsCert,
     key: program.httpsKey,
-    // important: we need this hack in both devel and prod mode (i.e., on cocalc-docker) or the hub
-    // websocket fails with nextjs 14.  This wasn't needed for nextjs 13 in prod mode.
     listenersHack:
       program.mode == "single-user" &&
       program.proxyServer &&
       program.nextServer &&
-      program.websocketServer,
+      program.websocketServer &&
+      process.env["NODE_ENV"] == "development",
   });
 
   // The express app create via initExpressApp above **assumes** that init_passport is done

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

@@ -111,7 +111,7 @@ export default function init(
   if (listenersHack) {
     // This is an insane horrible hack to fix https://github.com/sagemathinc/cocalc/issues/7067
     // The problem is that there are four separate websocket "upgrade" handlers when we are doing
-    // development, and nodejs just doesn't hav a good solution to multiple websocket handlers,
+    // development, and nodejs just doesn't have a good solution to multiple websocket handlers,
     // as explained here: https://github.com/nodejs/node/issues/6339
     // The four upgrade handlers are:
     //   - this proxy here
@@ -123,13 +123,19 @@ export default function init(
     // What's worse is that getEventListeners only seems to ever return *two*
     // listeners.  By extensive trial and error, it seems to return first the primus
     // listener, then the nextjs one.  I have no idea why the order is that way; I would
-    // expect the reverse.   And I don't know why this handler here isn't in the list.
+    // expect the reverse.  (Update: it's because nextjs uses a hack -- it only installs
+    // a listener once a request comes in. Until there is a request, nextjs does not have
+    // access to the server and can't mess with it.)
+    // And I don't know why this handler here isn't in the list.
     // In any case, once we get a failed request *and* we see there are at least two
     // other handlers (it's exactly two), we completely steal handling of the upgrade
     // event here.  We then call the appropriate other handler when needed.
     // I have no idea how the HMR reloader for that static webpack plays into this,
     // but it appears to just work for some reason.
 
+    // NOTE: I had to do something similar that is in packages/next/lib/init.js,
+    // and is NOT a hack.  That technique could probably be used to fix this properly.
+
     let listeners: any[] = [];
     handler = async (req, socket, head) => {
       logger.debug("Proxy websocket handling -- using listenersHack");

src/packages/hub/servers/app/next.ts

@@ -64,7 +64,7 @@ export default async function init(app: Application) {
         } catch (_err) {
           res.status(404).end();
         }
-      }
+      },
     );
 
     // 2: The download server -- just like raw, but files always get sent via download.
@@ -83,7 +83,7 @@ export default async function init(app: Application) {
         } catch (_err) {
           res.status(404).end();
         }
-      }
+      },
     );
 
     // 3: Redirects for backward compat; unfortunately there's slight
@@ -99,8 +99,10 @@ export default async function init(app: Application) {
 
   // The next.js server that serves everything else.
   winston.info(
-    "Now using next.js packages/share handler to handle all endpoints not otherwise handled"
+    "Now using next.js packages/share handler to handle all endpoints not otherwise handled",
   );
+
+  // nextjs listens on everything else
   app.all("*", handler);
 }
 

src/packages/next/lib/init.js

@@ -33,6 +33,7 @@ async function init({ basePath }) {
 
   winston.info(`creating next.js app with dev=${dev}`);
   const app = next({ dev, dir: join(__dirname, ".."), conf });
+
   const handle = app.getRequestHandler();
   winston.info("preparing next.js app...");
 
@@ -47,8 +48,33 @@ async function init({ basePath }) {
   // following workaround, which is just to explicitly init webpack
   // before it gets used in prepare below:
   require("next/dist/compiled/webpack/webpack").init(); // see comment above.
+
+  // app.prepare  sets app.upgradeHandler, etc. --
+  // see https://github.com/vercel/next.js/blob/canary/packages/next/src/server/next.ts#L276
   await app.prepare();
 
+  if (!dev) {
+    // The following is NOT a crazy a hack -- it's the result of me (ws)
+    // carefully reading nextjs source code for several hours.
+    // In production mode, we must completely disable the nextjs websocket upgrade
+    // handler, since it breaks allowing users to connect to the hub via a websocket,
+    // as it just kills all such connection immediately.  That's done via some new
+    // code in nextjs v14 that IMHO the author does not understand, as you can see here:
+    // https://github.com/vercel/next.js/blob/23eba22d02290cff0021a53f449f1d7e32a35e56/packages/next/src/server/lib/router-server.ts#L667
+    // where there is a comment "// TODO: allow upgrade requests to pages/app paths?".
+    // In dev mode we leave this, since it suppots hot module loading, though
+    // we use a hack (see packages/hub/proxy/handle-upgrade.ts) that involves
+    // removing listeners. That hack could probably be redone better by using
+    // app.upgradeHandler directly.
+    // To see the importance of this you must:
+    //   - build in prod mode (not dev, obviously)
+    //   - load the cocalc next landing page /
+    //   - then try to view /projects
+    // Without this fix, the websocket will disconnect. With this fix, the websocket works.
+    winston.info("patching upgrade handler");
+    app.upgradeHandler = () => {};
+  }
+
   winston.info("ready to handle requests:");
   return (req, res) => {
     winston.http(`req.url=${req.url}`);