Refactor incremental render request handling and improve error management

- Removed unnecessary bundle validation checks from the incremental render request flow.
- Enhanced the `handleIncrementalRenderRequest` function to directly call `handleRenderRequest`, streamlining the rendering process.
- Updated the `IncrementalRenderInitialRequest` type to support a more flexible structure for dependency timestamps.
- Improved error handling to capture unexpected errors during the rendering process, ensuring robust responses.
- Added cleanup logic in tests to restore mocks after each test case.

Author: AbanoubGhadban

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

@@ -34,7 +34,6 @@ import {
   getAssetPath,
   getBundleDirectory,
   deleteUploadedAssets,
-  validateBundlesExist,
 } from './shared/utils';
 import * as errorReporter from './shared/errorReporter';
 import { lock, unlock } from './shared/locks';
@@ -290,10 +289,6 @@ export default function run(config: Partial<Config>) {
   }>('/bundles/:bundleTimestamp/incremental-render/:renderRequestDigest', async (req, res) => {
     const { bundleTimestamp } = req.params;
 
-    // Perform protocol + auth checks as early as possible. For protocol check,
-    // we need the first NDJSON object; thus defer protocol/auth until first chunk is parsed.
-    // Headers and status will be set after validation passes to avoid premature 200 status.
-
     // Stream parser state
     let renderResult: Awaited<ReturnType<typeof handleIncrementalRenderRequest>> | null = null;
 
@@ -306,7 +301,10 @@ export default function run(config: Partial<Config>) {
           const tempReqBody = typeof obj === 'object' && obj !== null ? (obj as Record<string, unknown>) : {};
 
           // Protocol check
-          const protoResult = checkProtocolVersion({ ...req, body: tempReqBody } as unknown as FastifyRequest);
+          const protoResult = checkProtocolVersion({
+            ...req,
+            body: tempReqBody,
+          } as unknown as FastifyRequest);
           if (typeof protoResult === 'object') {
             return {
               response: protoResult,
@@ -315,28 +313,23 @@ export default function run(config: Partial<Config>) {
           }
 
           // Auth check
-          const authResult = authenticate({ ...req, body: tempReqBody } as unknown as FastifyRequest);
+          const authResult = authenticate({
+            ...req,
+            body: tempReqBody,
+          } as unknown as FastifyRequest);
           if (typeof authResult === 'object') {
             return {
               response: authResult,
               shouldContinue: false,
             };
           }
 
-          // Bundle validation
+          // Extract data for incremental render request
           const dependencyBundleTimestamps = extractBodyArrayField(
             tempReqBody as WithBodyArrayField<Record<string, unknown>, 'dependencyBundleTimestamps'>,
             'dependencyBundleTimestamps',
           );
-          const missingBundleError = await validateBundlesExist(bundleTimestamp, dependencyBundleTimestamps);
-          if (missingBundleError) {
-            return {
-              response: missingBundleError,
-              shouldContinue: false,
-            };
-          }
 
-          // All validation passed - get response stream
           const initial: IncrementalRenderInitialRequest = {
             renderingRequest: String((tempReqBody as { renderingRequest?: string }).renderingRequest ?? ''),
             bundleTimestamp,

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

@@ -1,5 +1,5 @@
-import { Readable } from 'stream';
 import type { ResponseResult } from '../shared/utils';
+import { handleRenderRequest } from './handleRenderRequest';
 
 export type IncrementalRenderSink = {
   /** Called for every subsequent NDJSON object after the first one */
@@ -13,7 +13,7 @@ export type IncrementalRenderSink = {
 export type IncrementalRenderInitialRequest = {
   renderingRequest: string;
   bundleTimestamp: string | number;
-  dependencyBundleTimestamps?: Array<string | number>;
+  dependencyBundleTimestamps?: string[] | number[];
 };
 
 export type IncrementalRenderResult = {
@@ -22,36 +22,64 @@ export type IncrementalRenderResult = {
 };
 
 /**
- * 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.
+ * Starts handling an incremental render request. This function:
+ * - Calls handleRenderRequest internally to handle all validation and VM execution
+ * - Returns the result from handleRenderRequest directly
+ * - Provides a sink for future incremental updates (to be implemented in next commit)
  */
-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: new Readable({
-        read() {
-          // No-op for now
+export async function handleIncrementalRenderRequest(
+  initial: IncrementalRenderInitialRequest,
+): Promise<IncrementalRenderResult> {
+  const { renderingRequest, bundleTimestamp, dependencyBundleTimestamps } = initial;
+
+  try {
+    // Call handleRenderRequest internally to handle all validation and VM execution
+    const renderResult = await handleRenderRequest({
+      renderingRequest,
+      bundleTimestamp,
+      dependencyBundleTimestamps,
+      providedNewBundles: undefined,
+      assetsToCopy: undefined,
+    });
+
+    // Return the result directly with a placeholder sink
+    return {
+      response: renderResult,
+      sink: {
+        add: () => {
+          /* no-op - will be implemented in next commit */
+        },
+        end: () => {
+          /* no-op - will be implemented in next commit */
+        },
+        abort: () => {
+          /* no-op - will be implemented in next commit */
         },
-      }),
-    } as ResponseResult,
-    sink: {
-      add: () => {
-        /* no-op */
       },
-      end: () => {
-        /* no-op */
+    };
+  } catch (error) {
+    // Handle any unexpected errors
+    const errorMessage = error instanceof Error ? error.message : String(error);
+
+    return {
+      response: {
+        status: 500,
+        headers: { 'Cache-Control': 'no-cache, no-store, max-age=0, must-revalidate' },
+        data: errorMessage,
       },
-      abort: () => {
-        /* no-op */
+      sink: {
+        add: () => {
+          /* no-op */
+        },
+        end: () => {
+          /* no-op */
+        },
+        abort: () => {
+          /* no-op */
+        },
       },
-    },
-  });
+    };
+  }
 }
 
 export type { ResponseResult };

react_on_rails_pro/packages/node-renderer/tests/incrementalRender.test.ts

@@ -211,7 +211,7 @@ describe('incremental render NDJSON endpoint', () => {
    */
   const createStreamingResponsePromise = (req: http.ClientRequest) => {
     const receivedChunks: string[] = [];
-    
+
     const promise = new Promise<{ statusCode: number; streamedData: string[] }>((resolve, reject) => {
       req.on('response', (res) => {
         res.on('data', (chunk: Buffer) => {
@@ -236,6 +236,10 @@ describe('incremental render NDJSON endpoint', () => {
     return { promise, receivedChunks };
   };
 
+  afterEach(() => {
+    jest.restoreAllMocks();
+  });
+
   beforeAll(async () => {
     await app.ready();
     await app.listen({ port: 0 });