Handle out-of-memory errors in the worker thread (#4273)

its/plugin/sonarlint-tests/src/test/java/SonarLintTest.java

@@ -122,9 +122,9 @@ private static boolean usingEmbeddedNode() {
   @Test
   void should_start_node_server_once() throws Exception {
     analyze(FILE_PATH, "");
-    assertThat(logs).doesNotContain("the bridge server is up, no need to start.");
+    assertThat(logs).doesNotContain("The bridge server is up, no need to start.");
     analyze(FILE_PATH, "");
-    assertThat(logs).contains("the bridge server is up, no need to start.");
+    assertThat(logs).contains("The bridge server is up, no need to start.");
   }
 
   @Test

its/plugin/tests/src/test/java/com/sonar/javascript/it/plugin/YamlAnalysisTest.java

@@ -64,7 +64,7 @@ void single_line_inline_aws_lambda_for_js() throws IOException {
     assertThat(issuesList)
       .extracting(Issue::getLine, Issue::getRule)
       .containsExactlyInAnyOrder(tuple(12, "javascript:S3923"));
-    assertThat(result.getLogsLines(log -> log.contains("Starting Node.js process"))).hasSize(1);
+    assertThat(result.getLogsLines(log -> log.contains("Creating Node.js process"))).hasSize(1);
     // assertPerfMonitoringAvailable(perfMonitoringDir);
   }
 
@@ -89,6 +89,6 @@ void dont_start_eslint_bridge_for_yaml_without_nodejs_aws() {
 
     OrchestratorStarter.setProfiles(projectKey, Map.of("eslint-based-rules-profile", "js"));
     BuildResult result = orchestrator.executeBuild(build);
-    assertThat(result.getLogsLines(log -> log.contains("Starting Node.js process"))).hasSize(0);
+    assertThat(result.getLogsLines(log -> log.contains("Creating Node.js process"))).hasSize(0);
   }
 }

packages/bridge/src/errors/middleware.ts

@@ -20,6 +20,7 @@
 import express from 'express';
 import { ErrorCode } from '@sonar/shared/errors';
 import { JsTsAnalysisOutput } from '@sonar/jsts';
+import { error } from '@sonar/shared/helpers';
 
 /**
  * Express.js middleware for handling error while serving requests.
@@ -32,12 +33,12 @@ import { JsTsAnalysisOutput } from '@sonar/jsts';
  * @see https://expressjs.com/en/guide/error-handling.html
  */
 export function errorMiddleware(
-  error: any,
+  err: any,
   _request: express.Request,
   response: express.Response,
   _next: express.NextFunction,
 ) {
-  const { code, message, stack, data } = error;
+  const { code, message, stack, data } = err;
   switch (code) {
     case ErrorCode.Parsing:
       response.json({
@@ -59,7 +60,7 @@ export function errorMiddleware(
       });
       break;
     default:
-      console.error(stack);
+      error(stack);
       response.json({ error: message });
       break;
   }

packages/bridge/src/memory.ts

@@ -0,0 +1,78 @@
+/*
+ * SonarQube JavaScript Plugin
+ * Copyright (C) 2011-2023 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+import * as v8 from 'v8';
+import * as os from 'os';
+import fs from 'fs';
+import { error, info, warn } from '@sonar/shared/helpers';
+
+const MB = 1024 * 1024;
+
+export function logMemoryConfiguration() {
+  const osMem = Math.floor(os.totalmem() / MB);
+  const heapSize = getHeapSize();
+  const dockerMemLimit = readDockerMemoryLimit();
+  const dockerMem = dockerMemLimit ? `Docker mem: ${dockerMemLimit} MB. ` : '';
+  info(`OS memory ${osMem} MB. ${dockerMem}Node.js heap size limit: ${heapSize} MB.`);
+  if (heapSize > osMem) {
+    warn(
+      `Node.js heap size limit ${heapSize} is higher than available memory ${osMem}. Check your configuration of sonar.javascript.node.maxspace`,
+    );
+  }
+}
+
+function readDockerMemoryLimit() {
+  try {
+    const mem = Number.parseInt(fs.readFileSync('/sys/fs/cgroup/memory.max', { encoding: 'utf8' }));
+    if (Number.isInteger(mem)) {
+      return mem;
+    }
+  } catch (e) {
+    // probably not a docker env
+  }
+  return undefined;
+}
+
+function getHeapSize() {
+  return Math.floor(v8.getHeapStatistics().heap_size_limit / MB);
+}
+
+export function logMemoryError(err: any) {
+  switch (err?.code) {
+    case 'ERR_WORKER_OUT_OF_MEMORY':
+      error(
+        `The analysis will stop due to the Node.js process running out of memory (heap size limit ${getHeapSize()} MB)`,
+      );
+      error(
+        `You can see how Node.js heap usage evolves during analysis with "sonar.javascript.node.debugMemory=true"`,
+      );
+      error(
+        'Try setting "sonar.javascript.node.maxspace" to a higher value to increase Node.js heap size limit',
+      );
+      error(
+        'If the problem persists, please report the issue at https://community.sonarsource.com',
+      );
+      break;
+    default:
+      error(`The analysis will stop due to an unexpected error: ${err}`);
+      error('Please report the issue at https://community.sonarsource.com');
+      break;
+  }
+}

packages/bridge/src/server.ts

@@ -17,6 +17,7 @@
  * along with this program; if not, write to the Free Software Foundation,
  * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */
+
 /**
  * `module-alias` must be imported first for module aliasing to work.
  */
@@ -27,13 +28,11 @@ import http from 'http';
 import path from 'path';
 import router from './router';
 import { errorMiddleware } from './errors';
-import { debug, getContext, info, warn } from '@sonar/shared/helpers';
+import { debug, getContext } from '@sonar/shared/helpers';
 import { timeoutMiddleware } from './timeout';
 import { AddressInfo } from 'net';
-import * as v8 from 'v8';
-import * as os from 'os';
 import { Worker } from 'worker_threads';
-import fs from 'fs';
+import { logMemoryConfiguration, logMemoryError } from './memory';
 
 /**
  * The maximum request body size
@@ -49,33 +48,6 @@ const MAX_REQUEST_SIZE = '50mb';
  */
 const SHUTDOWN_TIMEOUT = 15_000;
 
-const MB = 1024 * 1024;
-
-function logMemoryConfiguration() {
-  const osMem = Math.floor(os.totalmem() / MB);
-  const heapSize = Math.floor(v8.getHeapStatistics().heap_size_limit / MB);
-  const dockerMemLimit = readDockerMemoryLimit();
-  const dockerMem = dockerMemLimit ? `Docker mem: ${dockerMemLimit} MB. ` : '';
-  info(`OS memory ${osMem} MB. ${dockerMem}Node.js heap size limit: ${heapSize} MB.`);
-  if (heapSize > osMem) {
-    warn(
-      `Node.js heap size limit ${heapSize} is higher than available memory ${osMem}. Check your configuration of sonar.javascript.node.maxspace`,
-    );
-  }
-}
-
-function readDockerMemoryLimit() {
-  try {
-    const mem = Number.parseInt(fs.readFileSync('/sys/fs/cgroup/memory.max', { encoding: 'utf8' }));
-    if (Number.isInteger(mem)) {
-      return mem;
-    }
-  } catch (e) {
-    // probably not a docker env
-  }
-  return undefined;
-}
-
 /**
  * A pool of a single worker thread
  *
@@ -128,6 +100,19 @@ export function start(
 
     worker.on('error', err => {
       debug(`The worker thread failed: ${err}`);
+
+      logMemoryError(err);
+
+      /**
+       * At this point, the worker thread can no longer respond to any request from the plugin.
+       * However, existing requests are stalled until they time out. Since the bridge server is
+       * about to be shut down in an unexpected manner anyway, we can close all connections and
+       * avoid waiting unnecessarily for them to eventually close.
+       */
+      server.closeAllConnections();
+
+      debug('Shutting down the bridge server due to failure');
+      shutdown();
     });
 
     const app = express();

packages/bridge/tests/memory.test.ts

@@ -0,0 +1,57 @@
+/*
+ * SonarQube JavaScript Plugin
+ * Copyright (C) 2011-2023 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+import { logMemoryError } from '../src/memory';
+
+describe('logMemoryError', () => {
+  it('should log out-of-memory troubleshooting guide', () => {
+    console.error = jest.fn();
+
+    logMemoryError({ code: 'ERR_WORKER_OUT_OF_MEMORY' });
+
+    expect(console.error).toHaveBeenCalledWith(
+      expect.stringContaining(
+        `The analysis will stop due to the Node\.js process running out of memory`,
+      ),
+    );
+    expect(console.error).toHaveBeenCalledWith(
+      'You can see how Node.js heap usage evolves during analysis with "sonar.javascript.node.debugMemory=true"',
+    );
+    expect(console.error).toHaveBeenCalledWith(
+      'Try setting "sonar.javascript.node.maxspace" to a higher value to increase Node.js heap size limit',
+    );
+    expect(console.error).toHaveBeenCalledWith(
+      'If the problem persists, please report the issue at https://community.sonarsource.com',
+    );
+  });
+
+  it('should log default troubleshooting guide', () => {
+    console.error = jest.fn();
+
+    logMemoryError('something failed');
+
+    expect(console.error).toHaveBeenCalledWith(
+      'The analysis will stop due to an unexpected error: something failed',
+    );
+    expect(console.error).toHaveBeenCalledWith(
+      'Please report the issue at https://community.sonarsource.com',
+    );
+  });
+});

packages/jsts/src/linter/issues/message.ts

@@ -20,6 +20,7 @@
 import { Linter, SourceCode } from 'eslint';
 import { transformFixes } from '../quickfixes';
 import { Issue } from './issue';
+import { error } from '@sonar/shared/helpers';
 
 /**
  * Converts an ESLint message into a SonarQube issue
@@ -40,7 +41,7 @@ export function convertMessage(source: SourceCode, message: Linter.LintMessage):
    * happen because we lint ready SourceCode instances and not file contents.
    */
   if (!message.ruleId) {
-    console.error("Illegal 'null' ruleId for eslint issue");
+    error("Illegal 'null' ruleId for eslint issue");
     return null;
   }
   return {

packages/jsts/src/program/program.ts

@@ -30,7 +30,14 @@
 
 import path from 'path';
 import ts from 'typescript';
-import { addTsConfigIfDirectory, debug, readFileSync, toUnixPath } from '@sonar/shared/helpers';
+import {
+  addTsConfigIfDirectory,
+  debug,
+  error,
+  readFileSync,
+  toUnixPath,
+  warn,
+} from '@sonar/shared/helpers';
 import tmp from 'tmp';
 import { promisify } from 'util';
 import fs from 'fs/promises';
@@ -90,7 +97,7 @@ export function createProgramOptions(
   const config = ts.readConfigFile(tsConfig, parseConfigHost.readFile);
 
   if (config.error) {
-    console.error(`Failed to parse tsconfig: ${tsConfig} (${diagnosticToString(config.error)})`);
+    error(`Failed to parse tsconfig: ${tsConfig} (${diagnosticToString(config.error)})`);
     throw Error(diagnosticToString(config.error));
   }
 
@@ -153,7 +160,7 @@ export function createProgram(tsConfig: string, tsconfigContents?: string): Prog
   for (const reference of inputProjectReferences) {
     const sanitizedReference = addTsConfigIfDirectory(reference.path);
     if (!sanitizedReference) {
-      console.log(`WARN Skipping missing referenced tsconfig.json: ${reference.path}`);
+      warn(`Skipping missing referenced tsconfig.json: ${reference.path}`);
     } else {
       projectReferences.push(sanitizedReference);
     }

packages/shared/src/helpers/index.ts

@@ -18,7 +18,7 @@
  * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */
 export * from './context';
-export * from './debug';
+export * from './logging';
 export * from './files';
 export * from './language';
 export * from './escape';

packages/shared/src/helpers/debug.ts → packages/shared/src/helpers/logging.ts

@@ -29,6 +29,10 @@ export function debug(message: string) {
   console.log(`DEBUG ${message}`);
 }
 
+export function error(message: string) {
+  console.error(message);
+}
+
 export function info(message: string) {
   console.log(message);
 }

packages/shared/tests/helpers/debug.test.ts → packages/shared/tests/helpers/logging.test.ts

@@ -17,7 +17,8 @@
  * along with this program; if not, write to the Free Software Foundation,
  * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */
-import { debug, info, warn } from '../../src/helpers';
+
+import { debug, error, info, warn } from '../../src/helpers';
 
 describe('debug', () => {
   it('should log with a `DEBUG` prefix', () => {
@@ -27,6 +28,14 @@ describe('debug', () => {
   });
 });
 
+describe('error', () => {
+  it('should log to stderr', () => {
+    console.error = jest.fn();
+    error('hello, world!');
+    expect(console.error).toHaveBeenCalledWith(`hello, world!`);
+  });
+});
+
 describe('warn', () => {
   it('should log with a `WARN` prefix', () => {
     console.log = jest.fn();

sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/AbstractBridgeSensor.java

@@ -92,7 +92,7 @@ public void execute(SensorContext context) {
         );
       }
     } catch (Exception e) {
-      LOG.error("Failure during analysis, " + bridgeServer.getCommandInfo(), e);
+      LOG.error("Failure during analysis", e);
       if (contextUtils.failFast()) {
         throw new IllegalStateException(
           "Analysis failed (\"sonar.internal.analysis.failFast\"=true)",

sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/AnalysisWithProgram.java

@@ -149,9 +149,6 @@ private void analyze(InputFile file, @Nullable TsProgram tsProgram) throws IOExc
         "Analysis interrupted because the SensorContext is in cancelled state"
       );
     }
-    if (!bridgeServer.isAlive()) {
-      throw new IllegalStateException("the bridge server is not answering");
-    }
     var cacheStrategy = CacheStrategies.getStrategyFor(context, file);
     if (cacheStrategy.isAnalysisRequired()) {
       try {