Revert "Refactor incremental rendering logic and enhance bundle validation"

This reverts commit 26bac50ae9742e25ff75e5d1b27225a4495ef3dc.

Author: AbanoubGhadban

react_on_rails_pro/packages/node-renderer/src/worker.ts

@@ -34,11 +34,11 @@ import {
   getAssetPath,
   getBundleDirectory,
   deleteUploadedAssets,
+  validateBundlesExist,
 } from './shared/utils';
 import * as errorReporter from './shared/errorReporter';
 import { lock, unlock } from './shared/locks';
 import { startSsrRequestOptions, trace } from './shared/tracing';
-import { validateAndGetBundlePaths } from './worker/sharedRenderUtils';
 
 // Uncomment the below for testing timeouts:
 // import { delay } from './shared/utils';
@@ -323,16 +323,15 @@ export default function run(config: Partial<Config>) {
             };
           }
 
-          // Bundle validation using shared utility
+          // Bundle validation
           const dependencyBundleTimestamps = extractBodyArrayField(
             tempReqBody as WithBodyArrayField<Record<string, unknown>, 'dependencyBundleTimestamps'>,
             'dependencyBundleTimestamps',
           );
-          
-          const validationResult = await validateAndGetBundlePaths(bundleTimestamp, dependencyBundleTimestamps);
-          if (!validationResult.success) {
+          const missingBundleError = await validateBundlesExist(bundleTimestamp, dependencyBundleTimestamps);
+          if (missingBundleError) {
             return {
-              response: validationResult.error!,
+              response: missingBundleError,
               shouldContinue: false,
             };
           }

react_on_rails_pro/packages/node-renderer/src/worker/handleIncrementalRenderRequest.ts

@@ -1,7 +1,5 @@
 import { Readable } from 'stream';
-import { EventEmitter } from 'events';
 import type { ResponseResult } from '../shared/utils';
-import { validateAndGetBundlePaths, buildVMsForBundles, executeRenderInVM } from './sharedRenderUtils';
 
 export type IncrementalRenderSink = {
   /** Called for every subsequent NDJSON object after the first one */
@@ -24,156 +22,36 @@ export type IncrementalRenderResult = {
 };
 
 /**
- * Starts handling an incremental render request. This function:
- * - Creates an EventEmitter for handling updates
- * - Builds the VM if needed
- * - Executes the initial render request
- * - Returns both a stream that will be sent to the client and a sink for incoming chunks
+ * Starts handling an incremental render request. This function is intended to:
+ * - Initialize any resources needed to process the render
+ * - Return both a stream that will be sent to the client and a sink for incoming chunks
+ *
+ * NOTE: This is intentionally left unimplemented. Tests should mock this.
  */
-export async function handleIncrementalRenderRequest(
-  initial: IncrementalRenderInitialRequest,
-): Promise<IncrementalRenderResult> {
-  const { renderingRequest, bundleTimestamp, dependencyBundleTimestamps } = initial;
-
-  // Create event emitter for this specific request
-  const updateEmitter = new EventEmitter();
-
-  // Validate bundles and get paths
-  const validationResult = await validateAndGetBundlePaths(bundleTimestamp, dependencyBundleTimestamps);
-  if (!validationResult.success || !validationResult.bundleFilePath) {
-    return {
-      response: validationResult.error || {
-        status: 500,
-        headers: { 'Cache-Control': 'no-cache, no-store, max-age=0, must-revalidate' },
-        data: 'Bundle validation failed',
-      },
-      sink: {
-        add: () => {
-          /* no-op */
-        },
-        end: () => {
-          /* no-op */
-        },
-        abort: () => {
-          /* no-op */
-        },
-      },
-    };
-  }
-
-  // Build VMs
-  const vmBuildResult = await buildVMsForBundles(
-    validationResult.bundleFilePath,
-    validationResult.dependencyBundleFilePaths || [],
-  );
-  if (!vmBuildResult.success) {
-    return {
-      response: vmBuildResult.error || {
-        status: 500,
-        headers: { 'Cache-Control': 'no-cache, no-store, max-age=0, must-revalidate' },
-        data: 'VM building failed',
-      },
-      sink: {
-        add: () => {
-          /* no-op */
-        },
-        end: () => {
-          /* no-op */
-        },
-        abort: () => {
-          /* no-op */
-        },
-      },
-    };
-  }
-
-  // Create the response stream
-  const responseStream = new Readable({
-    read() {
-      // No-op - data will be pushed via events
-    },
-  });
-
-  // Set up event listeners for the response stream
-  updateEmitter.on('update', (data: unknown) => {
-    // Push update data to the response stream
-    responseStream.push(`${JSON.stringify(data)}\n`);
-  });
-
-  updateEmitter.on('end', () => {
-    // End the response stream
-    responseStream.push(null);
-  });
-
-  updateEmitter.on('error', (error: unknown) => {
-    // Handle error and end stream
-    const errorMessage = error instanceof Error ? error.message : String(error);
-    responseStream.push(`{"error":"${errorMessage}"}\n`);
-    responseStream.push(null);
-  });
-
-  // Execute the initial render request with the update emitter
-  const executionResult = await executeRenderInVM(
-    renderingRequest,
-    validationResult.bundleFilePath,
-    updateEmitter,
-  );
-
-  // Handle the render result
-  if (executionResult.success && executionResult.result) {
-    // Initial render completed successfully
-    if (executionResult.result.data) {
-      const dataString =
-        typeof executionResult.result.data === 'string'
-          ? executionResult.result.data
-          : JSON.stringify(executionResult.result.data);
-      responseStream.push(`${dataString}\n`);
-    }
-  } else {
-    // Render failed
-    const errorMessage =
-      typeof executionResult.error?.data === 'string' ? executionResult.error.data : 'Unknown render error';
-    responseStream.push(`{"error":"${errorMessage}"}\n`);
-    responseStream.push(null);
-    return {
-      response: executionResult.error || {
-        status: 500,
-        headers: { 'Cache-Control': 'no-cache, no-store, max-age=0, must-revalidate' },
-        data: 'Render execution failed',
-      },
-      sink: {
-        add: () => {
-          /* no-op */
-        },
-        end: () => {
-          /* no-op */
-        },
-        abort: () => {
-          /* no-op */
-        },
-      },
-    };
-  }
-
-  return {
+export function handleIncrementalRenderRequest(initial: IncrementalRenderInitialRequest): Promise<IncrementalRenderResult> {
+  // Empty placeholder implementation. Real logic will be added later.
+  return Promise.resolve({
     response: {
       status: 200,
       headers: { 'Cache-Control': 'no-cache, no-store, max-age=0, must-revalidate' },
-      stream: responseStream,
-    },
+      stream: new Readable({
+        read() {
+          // No-op for now
+        },
+      }),
+    } as ResponseResult,
     sink: {
-      add: (chunk: unknown) => {
-        // Emit event when chunk arrives
-        updateEmitter.emit('update', chunk);
+      add: () => {
+        /* no-op */
       },
       end: () => {
-        updateEmitter.emit('end');
+        /* no-op */
       },
-      abort: (error: unknown) => {
-        updateEmitter.emit('error', error);
+      abort: () => {
+        /* no-op */
       },
     },
-  };
+  });
 }
 
 export type { ResponseResult };

react_on_rails_pro/packages/node-renderer/src/worker/handleRenderRequest.ts

@@ -5,6 +5,7 @@
  * @module worker/handleRenderRequest
  */
 
+import cluster from 'cluster';
 import path from 'path';
 import { mkdir } from 'fs/promises';
 import { lock, unlock } from '../shared/locks';
@@ -18,17 +19,15 @@ import {
   copyUploadedAssets,
   ResponseResult,
   moveUploadedAsset,
+  isReadableStream,
+  isErrorRenderResult,
   getRequestBundleFilePath,
   deleteUploadedAssets,
+  validateBundlesExist,
 } from '../shared/utils';
 import { getConfig } from '../shared/configBuilder';
-import { hasVMContextForBundle } from './vm';
-import {
-  validateAndGetBundlePaths,
-  buildVMsForBundles,
-  executeRenderInVM,
-  createRenderErrorResponse,
-} from './sharedRenderUtils';
+import * as errorReporter from '../shared/errorReporter';
+import { buildVM, hasVMContextForBundle, runInVM } from './vm';
 
 export type ProvidedNewBundle = {
   timestamp: string | number;
@@ -40,15 +39,36 @@ async function prepareResult(
   bundleFilePathPerTimestamp: string,
 ): Promise<ResponseResult> {
   try {
-    const executionResult = await executeRenderInVM(renderingRequest, bundleFilePathPerTimestamp);
+    const result = await runInVM(renderingRequest, bundleFilePathPerTimestamp, cluster);
+
+    let exceptionMessage = null;
+    if (!result) {
+      const error = new Error('INVALID NIL or NULL result for rendering');
+      exceptionMessage = formatExceptionMessage(renderingRequest, error, 'INVALID result for prepareResult');
+    } else if (isErrorRenderResult(result)) {
+      ({ exceptionMessage } = result);
+    }
+
+    if (exceptionMessage) {
+      return errorResponseResult(exceptionMessage);
+    }
 
-    if (!executionResult.success || !executionResult.result) {
-      return executionResult.error || errorResponseResult('Unknown error during render execution');
+    if (isReadableStream(result)) {
+      return {
+        headers: { 'Cache-Control': 'public, max-age=31536000' },
+        status: 200,
+        stream: result,
+      };
     }
 
-    return executionResult.result;
+    return {
+      headers: { 'Cache-Control': 'public, max-age=31536000' },
+      status: 200,
+      data: result,
+    };
   } catch (err) {
-    return createRenderErrorResponse(renderingRequest, err, 'Unknown error calling runInVM');
+    const exceptionMessage = formatExceptionMessage(renderingRequest, err, 'Unknown error calling runInVM');
+    return errorResponseResult(exceptionMessage);
   }
 }
 
@@ -173,6 +193,7 @@ export async function handleRenderRequest({
   assetsToCopy?: Asset[] | null;
 }): Promise<ResponseResult> {
   try {
+    // const bundleFilePathPerTimestamp = getRequestBundleFilePath(bundleTimestamp);
     const allBundleFilePaths = Array.from(
       new Set([...(dependencyBundleTimestamps ?? []), bundleTimestamp].map(getRequestBundleFilePath)),
     );
@@ -201,27 +222,25 @@ export async function handleRenderRequest({
       }
     }
 
-    // Validate bundles and get paths
-    const validationResult = await validateAndGetBundlePaths(bundleTimestamp, dependencyBundleTimestamps);
-    if (!validationResult.success || !validationResult.bundleFilePath) {
-      return validationResult.error || errorResponseResult('Bundle validation failed');
+    // Check if the bundle exists:
+    const missingBundleError = await validateBundlesExist(bundleTimestamp, dependencyBundleTimestamps);
+    if (missingBundleError) {
+      return missingBundleError;
     }
 
-    // Build VMs
-    const vmBuildResult = await buildVMsForBundles(
-      validationResult.bundleFilePath,
-      validationResult.dependencyBundleFilePaths || [],
-    );
-    if (!vmBuildResult.success) {
-      return vmBuildResult.error || errorResponseResult('VM building failed');
-    }
+    // The bundle exists, but the VM has not yet been created.
+    // Another worker must have written it or it was saved during deployment.
+    log.info('Bundle %s exists. Building VM for worker %s.', entryBundleFilePath, workerIdLabel());
+    await Promise.all(allBundleFilePaths.map((bundleFilePath) => buildVM(bundleFilePath)));
 
     return await prepareResult(renderingRequest, entryBundleFilePath);
   } catch (error) {
-    return createRenderErrorResponse(
+    const msg = formatExceptionMessage(
       renderingRequest,
       error,
       'Caught top level error in handleRenderRequest',
     );
+    errorReporter.message(msg);
+    return Promise.reject(error as Error);
   }
 }