fix(core): changes to the daemon in order to support windows (#7211)

packages/workspace/src/core/project-graph/daemon/exec/index.ts

@@ -1,8 +1,8 @@
-import { logger } from '@nrwl/devkit';
+import { logger, normalizePath } from '@nrwl/devkit';
 import { appRootPath } from '@nrwl/tao/src/utils/app-root';
 import { spawn, spawnSync } from 'child_process';
 import { ensureFileSync } from 'fs-extra';
-import { join, sep } from 'path';
+import { join } from 'path';
 import { dirSync } from 'tmp';
 import {
   DaemonJson,
@@ -17,9 +17,9 @@ export async function startInBackground(): Promise<void> {
    * starting the server, as well as providing a reference to where any subsequent
    * log files can be found.
    */
-  const tmpDirPrefix = `nx-daemon--${appRootPath.replace(
-    // Replace the occurrences of / on unix systems, the \ on windows, with a -
-    new RegExp(escapeRegExp(sep), 'g'),
+  const tmpDirPrefix = `nx-daemon--${normalizePath(appRootPath).replace(
+    // Replace the occurrences of / in the unix-style normalized path with a -
+    new RegExp(escapeRegExp('/'), 'g'),
     '-'
   )}`;
   const serverLogOutputDir = dirSync({
@@ -43,7 +43,7 @@ export async function startInBackground(): Promise<void> {
 
   try {
     const backgroundProcess = spawn(
-      'node',
+      process.execPath,
       ['./start.js', serverLogOutputFile],
       {
         cwd: __dirname,
@@ -64,8 +64,8 @@ export async function startInBackground(): Promise<void> {
      * Ensure the server is actually available to connect to via IPC before resolving
      */
     return new Promise((resolve) => {
-      const id = setInterval(() => {
-        if (isServerAvailable()) {
+      const id = setInterval(async () => {
+        if (await isServerAvailable()) {
           clearInterval(id);
           resolve();
         }
@@ -80,7 +80,7 @@ export async function startInBackground(): Promise<void> {
 export function startInCurrentProcess(): void {
   logger.info(`NX Daemon Server - Starting in the current process...`);
 
-  spawnSync('node', ['./start.js'], {
+  spawnSync(process.execPath, ['./start.js'], {
     cwd: __dirname,
     stdio: 'inherit',
   });
@@ -89,7 +89,7 @@ export function startInCurrentProcess(): void {
 export function stop(): void {
   logger.info(`NX Daemon Server - Stopping...`);
 
-  spawnSync('node', ['./stop.js'], {
+  spawnSync(process.execPath, ['./stop.js'], {
     cwd: __dirname,
     stdio: 'inherit',
   });

packages/workspace/src/core/project-graph/daemon/server.spec.ts

@@ -59,13 +59,13 @@ describe('Daemon Server', () => {
 
   describe('isServerAvailable()', () => {
     it('should return true if the daemon server is available for connections', async () => {
-      expect(isServerAvailable()).toBe(false);
+      expect(await isServerAvailable()).toBe(false);
 
       await startServer({});
-      expect(isServerAvailable()).toBe(true);
+      expect(await isServerAvailable()).toBe(true);
 
       await stopServer();
-      expect(isServerAvailable()).toBe(false);
+      expect(await isServerAvailable()).toBe(false);
     });
   });
 

packages/workspace/src/core/project-graph/daemon/server.ts

@@ -118,10 +118,38 @@ function createAndSerializeProjectGraph(): string {
 }
 
 /**
- * For now we just invoke the existing `createProjectGraph()` utility and return the project
- * graph upon connection to the server
+ * We need to make sure that we instantiate the PerformanceObserver only once, otherwise
+ * we will end up with duplicate entries in the server logs.
  */
 let performanceObserver: PerformanceObserver | undefined;
+
+/**
+ * Create the server and register a connection callback.
+ *
+ * NOTE: It is important that we do not eagerly perform any work directly upon connection
+ * of a client.
+ *
+ * This is because there are two different scenarios for connection that we need to support:
+ * 1) Checking if the server is fundamentally available
+ * 2) Requesting the project graph to be built
+ *
+ * For case (1) we want the operation to be as fast as possible - we just need to know if the
+ * server exists to connect to, and so this is why we do not perform any work directly upon
+ * connection.
+ *
+ * For case (2) we have a simple known data payload which the client will send to the server
+ * in order to trigger the more expensive graph construction logic.
+ *
+ * The real reason we need have these two separate steps in which we decouple connection from
+ * the request to actually build the project graph is down to the fact that on Windows, the named
+ * pipe behaves subtly differently from Unix domain sockets in that when you check if it exists
+ * as if it were a file (e.g. using fs.existsSync() or fs.open() or fs.statSync() etc), the native
+ * code which runs behind the scenes on the OS will actually trigger a connection to the server.
+ * Therefore if we were to simply perform the graph creation upon connection we would end up
+ * repeating work and throwing `EPIPE` errors.
+ */
+const REQUEST_PROJECT_GRAPH_PAYLOAD = 'REQUEST_PROJECT_GRAPH_PAYLOAD';
+
 const server = createServer((socket) => {
   if (!performanceObserver) {
     performanceObserver = new PerformanceObserver((list) => {
@@ -129,11 +157,21 @@ const server = createServer((socket) => {
       // Slight indentation to improve readability of the overall log file
       serverLog(`  Time taken for '${entry.name}'`, `${entry.duration}ms`);
     });
-  }
     performanceObserver.observe({ entryTypes: ['measure'], buffered: false });
+  }
+
+  socket.on('data', (data) => {
+    /**
+     * If anything other than the known project graph creation request payload is sent to
+     * the server, we throw an error.
+     */
+    const payload = data.toString();
+    if (payload !== REQUEST_PROJECT_GRAPH_PAYLOAD) {
+      throw new Error(`Unsupported payload sent to daemon server: ${payload}`);
+    }
 
     performance.mark('server-connection');
-  serverLog('Connection Received');
+    serverLog('Client Request for Project Graph Received');
 
     const currentGitHead = gitRevParseHead(appRootPath);
 
@@ -196,7 +234,7 @@ const server = createServer((socket) => {
      * of expensive work on the next client request.
      *
      * For reference, on the very large test repo https://github.com/vsavkin/interstellar the project
-   * graph nxdeps.json file is about 24MB, so memory utilization should not be a huge concern.
+     * graph nxdeps.json file is about 32MB, so memory utilization should not be a huge concern.
      */
     cachedSerializedProjectGraph = serializedProjectGraph;
 
@@ -230,6 +268,7 @@ const server = createServer((socket) => {
       );
     });
   });
+});
 
 interface StartServerOptions {
   serverLogOutputFile?: string;
@@ -246,7 +285,7 @@ export async function startServer({
   }
   return new Promise((resolve) => {
     server.listen(fullOSSocketPath, () => {
-      serverLog(`Started`);
+      serverLog(`Started listening on: ${fullOSSocketPath}`);
       return resolve(server);
     });
   });
@@ -283,12 +322,28 @@ export function killSocketOrPath(): void {
   } catch {}
 }
 
-export function isServerAvailable(): boolean {
+/**
+ * As noted in the comments above the createServer() call, in order to reliably (meaning it works
+ * cross-platform) check whether or not the server is availabe to request a project graph from we
+ * need to actually attempt connecting to it.
+ *
+ * Because of the behavior of named pipes on Windows, we cannot simply treat them as a file and
+ * check for their existence on disk (unlike with Unix Sockets).
+ */
+export async function isServerAvailable(): Promise<boolean> {
   try {
-    statSync(fullOSSocketPath);
-    return true;
+    const socket = connect(fullOSSocketPath);
+    return new Promise((resolve) => {
+      socket.on('connect', () => {
+        socket.destroy();
+        resolve(true);
+      });
+      socket.on('error', () => {
+        resolve(false);
+      });
+    });
   } catch {
-    return false;
+    return Promise.resolve(false);
   }
 }
 
@@ -319,6 +374,14 @@ export async function getProjectGraphFromServer(): Promise<ProjectGraph> {
       return reject(new Error(errorMessage) || err);
     });
 
+    /**
+     * Immediately after connecting to the server we send it the known project graph creation
+     * request payload. See the notes above createServer() for more context as to why we explicitly
+     * request the graph from the client like this.
+     */
+    socket.on('connect', () => {
+      socket.write(REQUEST_PROJECT_GRAPH_PAYLOAD);
+
       let serializedProjectGraph = '';
       socket.on('data', (data) => {
         serializedProjectGraph += data.toString();
@@ -326,7 +389,9 @@ export async function getProjectGraphFromServer(): Promise<ProjectGraph> {
 
       socket.on('end', () => {
         try {
-        const projectGraph = JSON.parse(serializedProjectGraph) as ProjectGraph;
+          const projectGraph = JSON.parse(
+            serializedProjectGraph
+          ) as ProjectGraph;
           logger.info('NX Daemon Client - Resolved ProjectGraph');
           return resolve(projectGraph);
         } catch {
@@ -337,4 +402,5 @@ export async function getProjectGraphFromServer(): Promise<ProjectGraph> {
         }
       });
     });
+  });
 }

packages/workspace/src/core/project-graph/project-graph.ts

@@ -175,7 +175,7 @@ export async function createProjectGraphAsync(
     );
   }
 
-  if (!isServerAvailable()) {
+  if (!(await isServerAvailable())) {
     logger.warn(
       '\nWARNING: You set NX_DAEMON=true but the Daemon Server is not running. Starting Daemon Server in the background...'
     );