Skip to content

Commit 8b10a30

Browse files
Refactor incremental rendering logic and enhance bundle validation
- Introduced `validateAndGetBundlePaths` and `buildVMsForBundles` functions to streamline bundle validation and VM building processes. - Updated `handleIncrementalRenderRequest` and `handleRenderRequest` to utilize the new validation and VM building functions, improving code clarity and maintainability. - Enhanced error handling for rendering execution and added support for incremental updates using an EventEmitter. - Created a new `sharedRenderUtils` module to encapsulate shared rendering logic, promoting code reuse and organization.
1 parent 425f107 commit 8b10a30

File tree

6 files changed

+445
-417
lines changed

6 files changed

+445
-417
lines changed

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,11 @@ import {
3434
getAssetPath,
3535
getBundleDirectory,
3636
deleteUploadedAssets,
37-
validateBundlesExist,
3837
} from './shared/utils';
3938
import * as errorReporter from './shared/errorReporter';
4039
import { lock, unlock } from './shared/locks';
4140
import { startSsrRequestOptions, trace } from './shared/tracing';
41+
import { validateAndGetBundlePaths } from './worker/sharedRenderUtils';
4242

4343
// Uncomment the below for testing timeouts:
4444
// import { delay } from './shared/utils';
@@ -323,15 +323,16 @@ export default function run(config: Partial<Config>) {
323323
};
324324
}
325325

326-
// Bundle validation
326+
// Bundle validation using shared utility
327327
const dependencyBundleTimestamps = extractBodyArrayField(
328328
tempReqBody as WithBodyArrayField<Record<string, unknown>, 'dependencyBundleTimestamps'>,
329329
'dependencyBundleTimestamps',
330330
);
331-
const missingBundleError = await validateBundlesExist(bundleTimestamp, dependencyBundleTimestamps);
332-
if (missingBundleError) {
331+
332+
const validationResult = await validateAndGetBundlePaths(bundleTimestamp, dependencyBundleTimestamps);
333+
if (!validationResult.success) {
333334
return {
334-
response: missingBundleError,
335+
response: validationResult.error!,
335336
shouldContinue: false,
336337
};
337338
}
Lines changed: 142 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import { Readable } from 'stream';
2+
import { EventEmitter } from 'events';
23
import type { ResponseResult } from '../shared/utils';
4+
import { validateAndGetBundlePaths, buildVMsForBundles, executeRenderInVM } from './sharedRenderUtils';
35

46
export type IncrementalRenderSink = {
57
/** Called for every subsequent NDJSON object after the first one */
@@ -22,36 +24,156 @@ export type IncrementalRenderResult = {
2224
};
2325

2426
/**
25-
* Starts handling an incremental render request. This function is intended to:
26-
* - Initialize any resources needed to process the render
27-
* - Return both a stream that will be sent to the client and a sink for incoming chunks
28-
*
29-
* NOTE: This is intentionally left unimplemented. Tests should mock this.
27+
* Starts handling an incremental render request. This function:
28+
* - Creates an EventEmitter for handling updates
29+
* - Builds the VM if needed
30+
* - Executes the initial render request
31+
* - Returns both a stream that will be sent to the client and a sink for incoming chunks
3032
*/
31-
export function handleIncrementalRenderRequest(initial: IncrementalRenderInitialRequest): Promise<IncrementalRenderResult> {
32-
// Empty placeholder implementation. Real logic will be added later.
33-
return Promise.resolve({
33+
export async function handleIncrementalRenderRequest(
34+
initial: IncrementalRenderInitialRequest,
35+
): Promise<IncrementalRenderResult> {
36+
const { renderingRequest, bundleTimestamp, dependencyBundleTimestamps } = initial;
37+
38+
// Create event emitter for this specific request
39+
const updateEmitter = new EventEmitter();
40+
41+
// Validate bundles and get paths
42+
const validationResult = await validateAndGetBundlePaths(bundleTimestamp, dependencyBundleTimestamps);
43+
if (!validationResult.success || !validationResult.bundleFilePath) {
44+
return {
45+
response: validationResult.error || {
46+
status: 500,
47+
headers: { 'Cache-Control': 'no-cache, no-store, max-age=0, must-revalidate' },
48+
data: 'Bundle validation failed',
49+
},
50+
sink: {
51+
add: () => {
52+
/* no-op */
53+
},
54+
end: () => {
55+
/* no-op */
56+
},
57+
abort: () => {
58+
/* no-op */
59+
},
60+
},
61+
};
62+
}
63+
64+
// Build VMs
65+
const vmBuildResult = await buildVMsForBundles(
66+
validationResult.bundleFilePath,
67+
validationResult.dependencyBundleFilePaths || [],
68+
);
69+
if (!vmBuildResult.success) {
70+
return {
71+
response: vmBuildResult.error || {
72+
status: 500,
73+
headers: { 'Cache-Control': 'no-cache, no-store, max-age=0, must-revalidate' },
74+
data: 'VM building failed',
75+
},
76+
sink: {
77+
add: () => {
78+
/* no-op */
79+
},
80+
end: () => {
81+
/* no-op */
82+
},
83+
abort: () => {
84+
/* no-op */
85+
},
86+
},
87+
};
88+
}
89+
90+
// Create the response stream
91+
const responseStream = new Readable({
92+
read() {
93+
// No-op - data will be pushed via events
94+
},
95+
});
96+
97+
// Set up event listeners for the response stream
98+
updateEmitter.on('update', (data: unknown) => {
99+
// Push update data to the response stream
100+
responseStream.push(`${JSON.stringify(data)}\n`);
101+
});
102+
103+
updateEmitter.on('end', () => {
104+
// End the response stream
105+
responseStream.push(null);
106+
});
107+
108+
updateEmitter.on('error', (error: unknown) => {
109+
// Handle error and end stream
110+
const errorMessage = error instanceof Error ? error.message : String(error);
111+
responseStream.push(`{"error":"${errorMessage}"}\n`);
112+
responseStream.push(null);
113+
});
114+
115+
// Execute the initial render request with the update emitter
116+
const executionResult = await executeRenderInVM(
117+
renderingRequest,
118+
validationResult.bundleFilePath,
119+
updateEmitter,
120+
);
121+
122+
// Handle the render result
123+
if (executionResult.success && executionResult.result) {
124+
// Initial render completed successfully
125+
if (executionResult.result.data) {
126+
const dataString =
127+
typeof executionResult.result.data === 'string'
128+
? executionResult.result.data
129+
: JSON.stringify(executionResult.result.data);
130+
responseStream.push(`${dataString}\n`);
131+
}
132+
} else {
133+
// Render failed
134+
const errorMessage =
135+
typeof executionResult.error?.data === 'string' ? executionResult.error.data : 'Unknown render error';
136+
responseStream.push(`{"error":"${errorMessage}"}\n`);
137+
responseStream.push(null);
138+
return {
139+
response: executionResult.error || {
140+
status: 500,
141+
headers: { 'Cache-Control': 'no-cache, no-store, max-age=0, must-revalidate' },
142+
data: 'Render execution failed',
143+
},
144+
sink: {
145+
add: () => {
146+
/* no-op */
147+
},
148+
end: () => {
149+
/* no-op */
150+
},
151+
abort: () => {
152+
/* no-op */
153+
},
154+
},
155+
};
156+
}
157+
158+
return {
34159
response: {
35160
status: 200,
36161
headers: { 'Cache-Control': 'no-cache, no-store, max-age=0, must-revalidate' },
37-
stream: new Readable({
38-
read() {
39-
// No-op for now
40-
},
41-
}),
42-
} as ResponseResult,
162+
stream: responseStream,
163+
},
43164
sink: {
44-
add: () => {
45-
/* no-op */
165+
add: (chunk: unknown) => {
166+
// Emit event when chunk arrives
167+
updateEmitter.emit('update', chunk);
46168
},
47169
end: () => {
48-
/* no-op */
170+
updateEmitter.emit('end');
49171
},
50-
abort: () => {
51-
/* no-op */
172+
abort: (error: unknown) => {
173+
updateEmitter.emit('error', error);
52174
},
53175
},
54-
});
176+
};
55177
}
56178

57179
export type { ResponseResult };

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

Lines changed: 25 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
* @module worker/handleRenderRequest
66
*/
77

8-
import cluster from 'cluster';
98
import path from 'path';
109
import { mkdir } from 'fs/promises';
1110
import { lock, unlock } from '../shared/locks';
@@ -19,15 +18,17 @@ import {
1918
copyUploadedAssets,
2019
ResponseResult,
2120
moveUploadedAsset,
22-
isReadableStream,
23-
isErrorRenderResult,
2421
getRequestBundleFilePath,
2522
deleteUploadedAssets,
26-
validateBundlesExist,
2723
} from '../shared/utils';
2824
import { getConfig } from '../shared/configBuilder';
29-
import * as errorReporter from '../shared/errorReporter';
30-
import { buildVM, hasVMContextForBundle, runInVM } from './vm';
25+
import { hasVMContextForBundle } from './vm';
26+
import {
27+
validateAndGetBundlePaths,
28+
buildVMsForBundles,
29+
executeRenderInVM,
30+
createRenderErrorResponse,
31+
} from './sharedRenderUtils';
3132

3233
export type ProvidedNewBundle = {
3334
timestamp: string | number;
@@ -39,36 +40,15 @@ async function prepareResult(
3940
bundleFilePathPerTimestamp: string,
4041
): Promise<ResponseResult> {
4142
try {
42-
const result = await runInVM(renderingRequest, bundleFilePathPerTimestamp, cluster);
43-
44-
let exceptionMessage = null;
45-
if (!result) {
46-
const error = new Error('INVALID NIL or NULL result for rendering');
47-
exceptionMessage = formatExceptionMessage(renderingRequest, error, 'INVALID result for prepareResult');
48-
} else if (isErrorRenderResult(result)) {
49-
({ exceptionMessage } = result);
50-
}
51-
52-
if (exceptionMessage) {
53-
return errorResponseResult(exceptionMessage);
54-
}
43+
const executionResult = await executeRenderInVM(renderingRequest, bundleFilePathPerTimestamp);
5544

56-
if (isReadableStream(result)) {
57-
return {
58-
headers: { 'Cache-Control': 'public, max-age=31536000' },
59-
status: 200,
60-
stream: result,
61-
};
45+
if (!executionResult.success || !executionResult.result) {
46+
return executionResult.error || errorResponseResult('Unknown error during render execution');
6247
}
6348

64-
return {
65-
headers: { 'Cache-Control': 'public, max-age=31536000' },
66-
status: 200,
67-
data: result,
68-
};
49+
return executionResult.result;
6950
} catch (err) {
70-
const exceptionMessage = formatExceptionMessage(renderingRequest, err, 'Unknown error calling runInVM');
71-
return errorResponseResult(exceptionMessage);
51+
return createRenderErrorResponse(renderingRequest, err, 'Unknown error calling runInVM');
7252
}
7353
}
7454

@@ -193,7 +173,6 @@ export async function handleRenderRequest({
193173
assetsToCopy?: Asset[] | null;
194174
}): Promise<ResponseResult> {
195175
try {
196-
// const bundleFilePathPerTimestamp = getRequestBundleFilePath(bundleTimestamp);
197176
const allBundleFilePaths = Array.from(
198177
new Set([...(dependencyBundleTimestamps ?? []), bundleTimestamp].map(getRequestBundleFilePath)),
199178
);
@@ -222,25 +201,27 @@ export async function handleRenderRequest({
222201
}
223202
}
224203

225-
// Check if the bundle exists:
226-
const missingBundleError = await validateBundlesExist(bundleTimestamp, dependencyBundleTimestamps);
227-
if (missingBundleError) {
228-
return missingBundleError;
204+
// Validate bundles and get paths
205+
const validationResult = await validateAndGetBundlePaths(bundleTimestamp, dependencyBundleTimestamps);
206+
if (!validationResult.success || !validationResult.bundleFilePath) {
207+
return validationResult.error || errorResponseResult('Bundle validation failed');
229208
}
230209

231-
// The bundle exists, but the VM has not yet been created.
232-
// Another worker must have written it or it was saved during deployment.
233-
log.info('Bundle %s exists. Building VM for worker %s.', entryBundleFilePath, workerIdLabel());
234-
await Promise.all(allBundleFilePaths.map((bundleFilePath) => buildVM(bundleFilePath)));
210+
// Build VMs
211+
const vmBuildResult = await buildVMsForBundles(
212+
validationResult.bundleFilePath,
213+
validationResult.dependencyBundleFilePaths || [],
214+
);
215+
if (!vmBuildResult.success) {
216+
return vmBuildResult.error || errorResponseResult('VM building failed');
217+
}
235218

236219
return await prepareResult(renderingRequest, entryBundleFilePath);
237220
} catch (error) {
238-
const msg = formatExceptionMessage(
221+
return createRenderErrorResponse(
239222
renderingRequest,
240223
error,
241224
'Caught top level error in handleRenderRequest',
242225
);
243-
errorReporter.message(msg);
244-
return Promise.reject(error as Error);
245226
}
246227
}

0 commit comments

Comments
 (0)