Skip to content

Commit 4842440

Browse files
committed
refactor: trim comments to only add context not evident from code
Remove inline comments and JSDoc prose that restate the code or can be inferred from implementation. Keep minimal @param/@return type blocks.
1 parent 88c24e2 commit 4842440

1 file changed

Lines changed: 6 additions & 52 deletions

File tree

server/controllers/project.controller.js

Lines changed: 6 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ export async function projectExists(projectId) {
167167

168168
/**
169169
* @param {string} username
170-
* @param {string} projectId - the database id or the slug or the project
170+
* @param {string} projectId
171171
* @return {Promise<boolean>}
172172
*/
173173
export async function projectForUserExists(username, projectId) {
@@ -198,15 +198,12 @@ export async function getProjectForUser(username, projectId) {
198198
}
199199

200200
/**
201-
* Adds URLs referenced in <script> tags to the `files` array of the project
202-
* so that they can be downloaded along with other remote files from S3.
203201
* @param {object} project
204-
* @void - modifies the `project` parameter
205202
*/
206203
function bundleExternalLibs(project) {
207204
const indexHtml = project.files.find((file) => file.name === 'index.html');
208205
if (!indexHtml || !indexHtml.content) {
209-
return; // Gracefully handle missing index.html
206+
return;
210207
}
211208

212209
const { window } = new JSDOM(indexHtml.content);
@@ -218,7 +215,6 @@ function bundleExternalLibs(project) {
218215
const path = src.split('/');
219216
const filename = path[path.length - 1];
220217

221-
// Prevent duplicate external libs if downloaded multiple times
222218
if (project.files.some((f) => f.name === filename && f.url === src)) {
223219
return;
224220
}
@@ -234,42 +230,29 @@ function bundleExternalLibs(project) {
234230
}
235231

236232
/**
237-
* Helper function to get a readable stream from an S3 URL
238-
* Optimized to return stream handle quickly without waiting for data
239233
* @param {string} url - S3 URL
240234
* @return {Promise<Readable>}
241235
*/
242236
async function getStreamFromS3Url(url) {
243-
// Parse the S3 URL to get bucket and key
244237
const urlObj = new URL(url);
245238
let bucket;
246239
let key;
247240

248-
// Support different S3 URL formats
249241
if (urlObj.hostname.includes('s3')) {
250-
// Format: https://bucket-name.s3.region.amazonaws.com/key
251-
// or https://s3.region.amazonaws.com/bucket-name/key
252242
if (urlObj.hostname.startsWith('s3')) {
253-
// https://s3.region.amazonaws.com/bucket-name/key
254243
const pathParts = urlObj.pathname.split('/').filter(Boolean);
255244
[bucket] = pathParts;
256245
key = pathParts.slice(1).join('/');
257246
} else {
258-
// https://bucket-name.s3.region.amazonaws.com/key
259247
[bucket] = urlObj.hostname.split('.');
260248
key = urlObj.pathname.substring(1);
261249
}
262250

263-
// by nityam, Get S3 object stream - returns immediately with stream handle
264-
// Data is only fetched when the stream is consumed (by JSZip)
265251
const command = new GetObjectCommand({ Bucket: bucket, Key: key });
266252
const response = await s3Client.send(command);
267-
268-
// Ensure we return the stream, not buffer the response
269253
return response.Body;
270254
}
271255

272-
// Not an S3 URL, fall back to axios with streaming
273256
const response = await axios.get(url, {
274257
responseType: 'stream',
275258
timeout: 30000
@@ -278,32 +261,25 @@ async function getStreamFromS3Url(url) {
278261
}
279262

280263
/**
281-
* Recursively adds a file and all of its children to the JSZip instance using streaming.
282-
* Files are fetched sequentially to avoid memory overload.
283264
* @param {object} file
284265
* @param {Array<object>} files
285266
* @param {JSZip} zip
286-
* @return {Promise<void>} - modifies the `zip` parameter
267+
* @return {Promise<void>}
287268
*/
288269
async function addFileToZip(file, files, zip) {
289270
if (file.fileType === 'folder') {
290271
const folderZip = file.name === 'root' ? zip : zip.folder(file.name);
291-
// Process children sequentially to avoid fetching all files upfront
292272
await file.children.reduce(async (previousPromise, fileId) => {
293273
await previousPromise;
294274
const childFile = files.find((f) => f.id === fileId);
295275
return addFileToZip(childFile, files, folderZip);
296276
}, Promise.resolve());
297277
} else if (file.url) {
298278
try {
299-
// Check if this is an S3 URL
300279
if (file.url.includes('s3') && file.url.includes('amazonaws.com')) {
301-
// Use S3 streaming for S3 URLs
302-
// This gets the stream handle quickly - actual data is fetched by JSZip during generation
303280
const stream = await getStreamFromS3Url(file.url);
304281
zip.file(file.name, stream, { binary: true });
305282
} else {
306-
// For external URLs, use axios with streaming
307283
const response = await axios.get(file.url, {
308284
responseType: 'stream',
309285
timeout: 30000
@@ -312,11 +288,9 @@ async function addFileToZip(file, files, zip) {
312288
}
313289
} catch (e) {
314290
console.warn(`Failed to fetch file from ${file.url}:`, e.message);
315-
// Add empty file on error to prevent ZIP corruption
316291
zip.file(file.name, Buffer.alloc(0));
317292
}
318293
} else {
319-
// Regular file with inline content
320294
zip.file(file.name, file.content);
321295
}
322296
}
@@ -340,51 +314,39 @@ async function buildZip(project, req, res) {
340314

341315
bundleExternalLibs(project);
342316

343-
// Send headers immediately to prevent gateway timeout
344317
res.writeHead(200, {
345318
'Content-Type': 'application/zip',
346319
'Content-disposition': `attachment; filename=${zipFileName}`,
347320
'Transfer-Encoding': 'chunked'
348321
});
349322

350-
// Send periodic keepalive comments to prevent gateway timeout
351-
// while we're building the file list. ZIP format allows for this.
352323
let keepaliveCounter = 0;
353324
keepaliveInterval = setInterval(() => {
354-
// Write a comment to keep connection alive without corrupting ZIP
355-
// This prevents 60s gateway timeouts during file list building
356325
if (!res.writableEnded) {
357-
res.write(Buffer.alloc(0)); // Empty write to keep connection alive
326+
res.write(Buffer.alloc(0));
358327
keepaliveCounter++;
359328
if (keepaliveCounter % 10 === 0) {
360329
console.log(
361330
`Keepalive: Building ZIP file list (${keepaliveCounter}s elapsed)...`
362331
);
363332
}
364333
}
365-
}, 1000); // Every second
334+
}, 1000);
366335

367-
// Sequentially add files - this avoids parallel S3 connection storms
368-
// but still requires getting all file references before streaming begins
369336
await addFileToZip(root, files, zip);
370337

371-
// Clear keepalive now that we're about to start streaming real data
372338
clearInterval(keepaliveInterval);
373339
keepaliveInterval = null;
374340

375-
// Generate ZIP stream with true end-to-end streaming
376-
// streamFiles: true means JSZip reads from our S3 streams on-demand
377341
const zipStream = zip.generateNodeStream({
378342
type: 'nodebuffer',
379343
streamFiles: true,
380344
compression: 'DEFLATE',
381345
compressionOptions: { level: 6 }
382346
});
383347

384-
// Pipe the ZIP stream to response - handles backpressure automatically
385348
zipStream.pipe(res);
386349

387-
// Handle stream errors
388350
zipStream.on('error', (err) => {
389351
console.error('Error streaming zip file:', err);
390352
if (!res.headersSent) {
@@ -397,25 +359,19 @@ async function buildZip(project, req, res) {
397359
}
398360
});
399361

400-
// Wait for the stream to finish
401362
await new Promise((resolve, reject) => {
402363
zipStream.on('end', resolve);
403364
zipStream.on('error', reject);
404365
res.on('error', reject);
405-
res.on('close', () => {
406-
// Client disconnected
407-
reject(new Error('Client disconnected'));
408-
});
366+
res.on('close', () => reject(new Error('Client disconnected')));
409367
});
410368
} catch (err) {
411369
console.error('Error building zip file:', err);
412370

413-
// Clean up keepalive if still running
414371
if (keepaliveInterval) {
415372
clearInterval(keepaliveInterval);
416373
}
417374

418-
// Only send error if response hasn't been sent yet
419375
if (!res.headersSent) {
420376
res.status(500).json({
421377
success: false,
@@ -434,11 +390,9 @@ export async function downloadProjectAsZip(req, res) {
434390
res.status(404).send({ message: 'Project with that id does not exist' });
435391
return;
436392
}
437-
// Await buildZip to ensure it completes before the function returns
438393
await buildZip(project, req, res);
439394
} catch (err) {
440395
console.error('Error in downloadProjectAsZip:', err);
441-
// Only send error if response hasn't been sent yet
442396
if (!res.headersSent) {
443397
res.status(500).json({
444398
success: false,

0 commit comments

Comments
 (0)