Fix infinite fork loop when node renderer worker fails to bind port

.github/workflows/claude-code-review.yml

@@ -7,6 +7,8 @@ on:
 jobs:
   claude-review:
     runs-on: ubuntu-latest
+    env:
+      HAS_CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN != '' }}
     permissions:
       contents: read
       pull-requests: write
@@ -21,6 +23,7 @@ jobs:
 
       - name: Run Claude Code Review
         id: claude-review
+        if: env.HAS_CLAUDE_CODE_OAUTH_TOKEN == 'true'
         uses: anthropics/claude-code-action@v1
         with:
           claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
@@ -43,3 +46,8 @@ jobs:
 
           claude_args: |
             --allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*)"
+
+      - name: Skip Claude review when token is unavailable
+        if: env.HAS_CLAUDE_CODE_OAUTH_TOKEN != 'true'
+        run: |
+          echo "Skipping Claude Code Review because CLAUDE_CODE_OAUTH_TOKEN is not available for this workflow context."

.lychee.toml

@@ -92,6 +92,7 @@ exclude = [
   '^https://devchat\.tv',  # Ruby Rogues podcast - times out in CI
   '^https?://chlg\.co',  # URL shortener - times out in CI
   '^https?://contributor-covenant\.org',  # Timeout in CI
+  '^https://www\.diva-portal\.org/smash/get/diva2:1903931/FULLTEXT01\.pdf$',  # Intermittent timeout in CI
 
   # ============================================================================
   # CHANGELOG COMPARE LINKS

NEWS.md

@@ -27,11 +27,11 @@ _A history of the news. A few bullets at the top will also show on the [README.m
 - 2016-11-03: Spoke at [LA Ruby: "React on Rails: Why, What, and How?"](http://www.meetup.com/laruby/events/234825187/). [Video and pictures in this article](https://blog.shakacode.com/my-react-on-rails-talk-at-the-la-ruby-rails-meetup-november-10-2016-eaaa83aff800#.ej6h4eglp).
 - 2016-12-20: New Video on Egghead.io: [Creating a component with React on Rails](https://egghead.io/lessons/react-creating-a-component-with-react-on-rails)
 - 2016-11-03: Spoke at [LA Ruby, 7pm, Thursday, November 10 in Venice, CA: "React on Rails: Why, What, and How?"](http://www.meetup.com/laruby/events/234825187/). [Video and pictures in this article](https://blog.shakacode.com/my-react-on-rails-talk-at-the-la-ruby-rails-meetup-november-10-2016-eaaa83aff800#.ej6h4eglp).
-- 2016-08-27: We now have a [Documentation Gitbook](https://shakacode.gitbooks.io/react-on-rails/content/) for improved readability & reference.
+- 2016-08-27: We now have a [Documentation Gitbook](https://reactonrails.com/docs/) for improved readability & reference.
 - 2016-08-21: v6.1 ships with several new features and bug fixes. See the [Changelog](CHANGELOG.md).
 - 2016-07-28: If you're doing server rendering, be sure to use mini_racer! See [issues/428](https://github.com/shakacode/react_on_rails/issues/428). It's supposedly much faster than `therubyracer`.
 
-- 2016-08-27: We now have a [Documentation Gitbook](https://shakacode.gitbooks.io/react-on-rails/content/) for improved readability & reference.
+- 2016-08-27: We now have a [Documentation Gitbook](https://reactonrails.com/docs/) for improved readability & reference.
 - 2016-08-21: v6.1 ships with several new features and bug fixes. See the [Changelog](CHANGELOG.md).
 - 2016-06-13: 6.0.4 shipped with a critical fix regarding a missing polyfill for `clearTimeout`, used by babel-polyfill.
 - 2016-06-06: 6.0.2 shipped with a critical fix if you are fragment caching the server generated React.

packages/react-on-rails-pro-node-renderer/src/master.ts

@@ -10,6 +10,7 @@ import { buildConfig, Config, logSanitizedConfig } from './shared/configBuilder.
 import restartWorkers from './master/restartWorkers.js';
 import * as errorReporter from './shared/errorReporter.js';
 import { getLicenseStatus } from './shared/licenseValidator.js';
+import { isWorkerStartupFailureMessage, type WorkerStartupFailureMessage } from './shared/workerMessages.js';
 
 const MILLISECONDS_IN_MINUTE = 60000;
 // How often to scan for orphaned upload directories.
@@ -77,21 +78,44 @@ export default function masterRun(runningConfig?: Partial<Config>) {
     })();
   }, ORPHAN_CLEANUP_INTERVAL_MS);
 
+  let isAbortingForStartupFailure = false;
+  let fatalStartupFailure: { workerId: number; failure: WorkerStartupFailureMessage } | null = null;
+
   for (let i = 0; i < workersCount; i += 1) {
     cluster.fork();
   }
 
+  cluster.on('message', (worker, message) => {
+    if (!isWorkerStartupFailureMessage(message) || isAbortingForStartupFailure) return;
+
+    isAbortingForStartupFailure = true;
+    fatalStartupFailure = { workerId: worker.id, failure: message };
+  });
+
   // Listen for dying workers:
   cluster.on('exit', (worker) => {
     if (worker.isScheduledRestart) {
       log.info('Restarting worker #%d on schedule', worker.id);
-    } else {
-      // TODO: Track last rendering request per worker.id
-      // TODO: Consider blocking a given rendering request if it kills a worker more than X times
-      const msg = `Worker ${worker.id} died UNEXPECTEDLY :(, restarting`;
+      cluster.fork();
+      return;
+    }
+
+    if (isAbortingForStartupFailure) {
+      const failure = fatalStartupFailure?.failure;
+      const msg =
+        failure?.code === 'EADDRINUSE'
+          ? `Node renderer startup failed: port ${failure.port} is already in use`
+          : `Node renderer startup failed in worker ${worker.id}: ${failure?.message || `exit code ${worker.process.exitCode}`}`;
+
       errorReporter.message(msg);
+      process.exit(1);
+      return;
     }
-    // Replace the dead worker:
+
+    // TODO: Track last rendering request per worker.id
+    // TODO: Consider blocking a given rendering request if it kills a worker more than X times
+    const msg = `Worker ${worker.id} died UNEXPECTEDLY :(, restarting`;
+    errorReporter.message(msg);
     cluster.fork();
   });
 

packages/react-on-rails-pro-node-renderer/src/shared/workerMessages.ts

@@ -0,0 +1,20 @@
+export const WORKER_STARTUP_FAILURE = 'NODE_RENDERER_WORKER_STARTUP_FAILURE' as const;
+
+export interface WorkerStartupFailureMessage {
+  type: typeof WORKER_STARTUP_FAILURE;
+  stage: 'listen';
+  code?: string;
+  errno?: number;
+  syscall?: string;
+  host: string;
+  port: number;
+  message: string;
+}
+
+export function isWorkerStartupFailureMessage(value: unknown): value is WorkerStartupFailureMessage {
+  return (
+    typeof value === 'object' &&
+    value !== null &&
+    (value as { type?: string }).type === WORKER_STARTUP_FAILURE
+  );
+}

packages/react-on-rails-pro-node-renderer/src/worker.ts

@@ -19,6 +19,7 @@ import checkProtocolVersion from './worker/checkProtocolVersionHandler.js';
 import authenticate from './worker/authHandler.js';
 import { handleRenderRequest, type ProvidedNewBundle } from './worker/handleRenderRequest.js';
 import handleGracefulShutdown from './worker/handleGracefulShutdown.js';
+import { WORKER_STARTUP_FAILURE, type WorkerStartupFailureMessage } from './shared/workerMessages.js';
 import {
   errorResponseResult,
   formatExceptionMessage,
@@ -465,6 +466,24 @@ export default function run(config: Partial<Config>) {
     app.listen({ port, host }, (err, address) => {
       if (err) {
         log.error({ err, host, port }, 'Node renderer failed to start');
+
+        if (cluster.isWorker && process.send) {
+          const startupFailure: WorkerStartupFailureMessage = {
+            type: WORKER_STARTUP_FAILURE,
+            stage: 'listen',
+            code: (err as NodeJS.ErrnoException).code,
+            errno: (err as NodeJS.ErrnoException).errno,
+            syscall: (err as NodeJS.ErrnoException).syscall,
+            host,
+            port,
+            message: err.message,
+          };
+          process.send(startupFailure, undefined, undefined, () => {
+            process.exit(1);
+          });
+          return;
+        }
+
         process.exit(1);
       }
       const workerName = worker ? `worker #${worker.id}` : 'master (single-process)';

packages/react-on-rails-pro-node-renderer/tests/masterStartupFailure.test.ts

@@ -0,0 +1,165 @@
+import {
+  WORKER_STARTUP_FAILURE,
+  isWorkerStartupFailureMessage,
+  type WorkerStartupFailureMessage,
+} from '../src/shared/workerMessages';
+
+/**
+ * These tests verify that the master process correctly handles the
+ * WORKER_STARTUP_FAILURE IPC message by aborting instead of reforking,
+ * while preserving the existing behavior for scheduled restarts and
+ * runtime crashes.
+ *
+ * We test the decision logic in isolation by simulating the cluster events
+ * that the master process listens to, rather than forking real workers.
+ */
+
+function buildStartupFailureMessage(
+  overrides: Partial<WorkerStartupFailureMessage> = {},
+): WorkerStartupFailureMessage {
+  return {
+    type: WORKER_STARTUP_FAILURE,
+    stage: 'listen',
+    code: 'EADDRINUSE',
+    errno: -48,
+    syscall: 'listen',
+    host: 'localhost',
+    port: 3800,
+    message: 'listen EADDRINUSE: address already in use :::3800',
+    ...overrides,
+  };
+}
+
+describe('master startup failure handling', () => {
+  let isAbortingForStartupFailure: boolean;
+  let fatalStartupFailure: { workerId: number; failure: WorkerStartupFailureMessage } | null;
+  let forkCount: number;
+  let exitCode: number | null;
+  let reportedMessages: string[];
+
+  // Simulated handlers matching master.ts logic
+  function handleMessage(worker: { id: number }, message: unknown) {
+    if (!isWorkerStartupFailureMessage(message) || isAbortingForStartupFailure) return;
+
+    isAbortingForStartupFailure = true;
+    fatalStartupFailure = { workerId: worker.id, failure: message };
+  }
+
+  function handleExit(worker: {
+    id: number;
+    isScheduledRestart?: boolean;
+    process: { exitCode: number | null };
+  }) {
+    if (worker.isScheduledRestart) {
+      forkCount += 1; // cluster.fork()
+      return;
+    }
+
+    if (isAbortingForStartupFailure) {
+      const failure = fatalStartupFailure?.failure;
+      const msg =
+        failure?.code === 'EADDRINUSE'
+          ? `Node renderer startup failed: port ${failure.port} is already in use`
+          : `Node renderer startup failed in worker ${worker.id}: ${failure?.message || `exit code ${worker.process.exitCode}`}`;
+
+      reportedMessages.push(msg);
+      exitCode = 1;
+      return;
+    }
+
+    const msg = `Worker ${worker.id} died UNEXPECTEDLY :(, restarting`;
+    reportedMessages.push(msg);
+    forkCount += 1; // cluster.fork()
+  }
+
+  beforeEach(() => {
+    isAbortingForStartupFailure = false;
+    fatalStartupFailure = null;
+    forkCount = 0;
+    exitCode = null;
+    reportedMessages = [];
+  });
+
+  it('aborts with clear message on EADDRINUSE startup failure', () => {
+    const worker = { id: 1, process: { exitCode: 1 } };
+    const failure = buildStartupFailureMessage();
+
+    // Worker sends startup failure message
+    handleMessage(worker, failure);
+    expect(isAbortingForStartupFailure).toBe(true);
+
+    // Worker exits
+    handleExit(worker);
+
+    expect(forkCount).toBe(0); // No refork
+    expect(exitCode).toBe(1);
+    expect(reportedMessages).toEqual(['Node renderer startup failed: port 3800 is already in use']);
+  });
+
+  it('aborts with generic message on non-EADDRINUSE startup failure', () => {
+    const worker = { id: 2, process: { exitCode: 1 } };
+    const failure = buildStartupFailureMessage({
+      code: 'EACCES',
+      message: 'listen EACCES: permission denied 0.0.0.0:80',
+    });
+
+    handleMessage(worker, failure);
+    handleExit(worker);
+
+    expect(forkCount).toBe(0);
+    expect(exitCode).toBe(1);
+    expect(reportedMessages).toEqual([
+      'Node renderer startup failed in worker 2: listen EACCES: permission denied 0.0.0.0:80',
+    ]);
+  });
+
+  it('deduplicates multiple workers failing simultaneously', () => {
+    const worker1 = { id: 1, process: { exitCode: 1 } };
+    const worker2 = { id: 2, process: { exitCode: 1 } };
+    const failure = buildStartupFailureMessage();
+
+    // Both workers send failure messages
+    handleMessage(worker1, failure);
+    handleMessage(worker2, failure);
+
+    // Only the first one is recorded
+    expect(fatalStartupFailure?.workerId).toBe(1);
+
+    // First worker exit triggers abort
+    handleExit(worker1);
+    expect(exitCode).toBe(1);
+    expect(forkCount).toBe(0);
+  });
+
+  it('reforks on scheduled restart', () => {
+    const worker = { id: 1, isScheduledRestart: true, process: { exitCode: 0 } };
+
+    handleExit(worker);
+
+    expect(forkCount).toBe(1);
+    expect(exitCode).toBeNull();
+    expect(reportedMessages).toHaveLength(0);
+  });
+
+  it('reforks on unexpected runtime crash without startup failure', () => {
+    const worker = { id: 3, process: { exitCode: 1 } };
+
+    // No startup failure message sent — this is a runtime crash
+    handleExit(worker);
+
+    expect(forkCount).toBe(1);
+    expect(exitCode).toBeNull();
+    expect(reportedMessages).toEqual(['Worker 3 died UNEXPECTEDLY :(, restarting']);
+  });
+
+  it('ignores non-startup-failure messages', () => {
+    const worker = { id: 1 };
+
+    handleMessage(worker, { type: 'SOME_OTHER_MESSAGE' });
+    handleMessage(worker, 'a string message');
+    handleMessage(worker, null);
+
+    expect(isAbortingForStartupFailure).toBe(false);
+    expect(fatalStartupFailure).toBeNull();
+  });
+});