solution by Anton-Kuchmasov · Pull Request #187 · mate-academy/node_compression-app

Anton-Kuchmasov · 2025-10-26T12:07:06Z

No description provided.

mateacademy-ai-mentor

Review Summary

I am requesting changes as there are a few critical issues that prevent the server from functioning correctly. You've done a great job setting up the server structure and using streams, but some key logic needs adjustment.

Here are the main points to address:

Race Condition: The fs.createReadStream call runs before fs.rename finishes, causing an error. All logic that depends on the renamed file needs to be moved inside the callback for fs.rename.
Server Crashes: In your error handling blocks, you call res.end() but don't return. This causes the code to continue executing, leading to a server crash. Please add a return after each res.end() in these paths.
Incorrect Filename: The server expects the file input field to be named filePicker, but the requirements specify file.
Incorrect Output Naming: The compressed file's name is being altered incorrectly (e.g., file.txt becomes file-txt.gzip). The requirement is to append the correct extension (like .gz) to the original filename, resulting in file.txt.gz.

Fixing these issues will get your project working as required. Keep up the great work!

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

mateacademy-ai-mentor · 2025-10-26T12:09:05Z

src/createServer.js

+
+        const compressionType = fields.compressionType[0];
+
+        const uploadedFile = files.filePicker[0];


The requirements state that the file input field should have the name file. The code is currently looking for filePicker, which will prevent it from finding the uploaded file in the form data.

mateacademy-ai-mentor · 2025-10-26T12:09:05Z

src/createServer.js

+        // double sanity for non-valid input
+        if (!compressionType || !uploadedFile) {
+          res.statusCode = 400;
+          res.end('Invalid input!');


After sending a response with res.end(), you should add a return statement. Without it, the code will continue executing and attempt to access properties of uploadedFile (on line 109), which could be undefined here, causing the server to crash.

mateacademy-ai-mentor · 2025-10-26T12:09:05Z

src/createServer.js

+          uploadedFile.originalFilename,
+        );
+
+        fs.rename(uploadedFile.filepath, correctUploadedFilePath, (_err) => {


fs.rename is an asynchronous function. The code on the next line (fs.createReadStream) will execute immediately without waiting for the rename operation to finish, leading to a race condition and likely a 'file not found' error. All the logic that depends on the renamed file (from line 119 onwards) should be placed inside the callback function of fs.rename to ensure it runs only after the file has been successfully renamed.

mateacademy-ai-mentor · 2025-10-26T12:09:05Z

src/createServer.js

+            const zippedFilePath =
+              path
+                .basename(correctUploadedFilePath)
+                .replaceAll(' ', '-')
+                .replaceAll(',', '-')
+                .replaceAll('.', '-') + '.gzip';


The requirements specify that the compressed filename should be the original filename with the new extension appended (e.g., file.txt becomes file.txt.gz).

This implementation incorrectly alters the original filename by using .replaceAll('.', '-'), which would turn file.txt into file-txt. Additionally, the required extension for gzip is .gz, but the code uses .gzip. This issue with replacing dots also affects the 'deflate' and 'br' cases.

mateacademy-ai-mentor

Review Summary

Great job addressing the race condition from the previous review! The code is much more robust now. However, this requires a few changes to address some critical issues that are preventing the server from working correctly.

The most important issue is with the stream handling logic. Response headers (like res.statusCode = 200 and res.setHeader) must be set before you call pipeline(). Otherwise, the server will crash. The pipeline function handles ending the response on its own, so you should remove res.end() from its callback.

Additionally, please make these two adjustments to meet the requirements:

The server should expect the uploaded file in files.file, not files.filePicker.
The file extension for gzip compression needs to be .gz instead of .gzip.

Once these are fixed, you'll be all set. Keep up the great work!

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

mateacademy-ai-mentor · 2025-10-26T12:22:48Z

src/createServer.js

+
+        const compressionType = fields.compressionType[0];
+
+        const uploadedFile = files.filePicker[0];


According to the task description, the file input field should have the name attribute set to file. The server should therefore expect the uploaded file data in files.file, not files.filePicker. This requirement was also mentioned in the previous review.

mateacademy-ai-mentor · 2025-10-26T12:22:48Z

src/createServer.js

+              // ERR_RESPONSE_HEADERS_MULTIPLE_CONTENT_DISPOSITION
+
+              const zippedFilePath =
+                path.basename(correctUploadedFilePath) + '.gzip';


The task requirements specify that the extension for gzip compression should be .gz. The current code uses .gzip, which is incorrect.

mateacademy-ai-mentor · 2025-10-26T12:22:48Z

src/createServer.js

+              pipeline(readStream, gzip, res, (error) => {
+                res.statusCode = 500;
+                res.end(`Error while piping: ${error}`);
+              });
+
+              // and if everything is OK, let's finalize
+              // our Response with all needed headers
+              res.statusCode = 200;
+
+              res.setHeader(
+                'Content-Disposition',
+                `attachment; filename=${zippedFilePath}`,
+              );
+              res.setHeader('Content-Type', mimeType);
+              res.setHeader('Content-Encoding', 'gzip');


This block has a critical logic error. Response headers (like status code and content-disposition) must be set before the pipeline starts sending the file. Please move all res.setHeader() calls and res.statusCode = 200 to be before the pipeline() call.

Additionally, the pipeline callback is misused. When pipeline finishes successfully, it has already ended the response stream (res). Calling res.end() again in the callback will crash the server. This callback should primarily be used for logging or cleanup, not for sending response data.

This same logic error is present in the deflate and br blocks as well.

mateacademy-ai-mentor

Review Summary

Excellent work getting so close to the final solution! I am approving this submission as an educational override to help you move forward, as there is one critical issue remaining.

The main thing to fix is the logic inside your pipeline callbacks. Currently, you are trying to modify the response (e.g., res.statusCode = 500 or res.end()) within the callback. This will cause the server to crash because the pipeline function automatically handles ending the response stream, both on success and on error. The callback's purpose is for server-side actions like logging, not for sending data to the client. This issue occurs for all three compression types.

Also, a minor suggestion: one of your variables holding a Brotli stream is named deflate, which could be confusing. Renaming it would improve clarity. You've done a fantastic job addressing the previous feedback!

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Rate AI review example ↩

mateacademy-ai-mentor · 2025-10-26T12:32:50Z

src/createServer.js

+              pipeline(readStream, gzip, res, (error) => {
+                res.statusCode = 500;
+                res.end(`Error while piping: ${error}`);
+              });


The logic inside this pipeline callback will cause the server to crash.

If the pipeline succeeds, pipeline automatically ends the response (res). Calling res.end() again will cause a 'write after end' error.

If the pipeline fails, headers have already been sent with status 200. Trying to set res.statusCode = 500 will cause a 'Cannot set headers' error.

The callback's purpose here is for logging or cleanup, not for modifying the response. Please remove the code that changes the response from this callback.

mateacademy-ai-mentor · 2025-10-26T12:32:50Z

src/createServer.js

+              pipeline(readStream, deflate, res, (error) => {
+                res.statusCode = 500;
+                res.end(`Error while piping: ${error}`);
+              });


This pipeline callback has the same issue as the one in the 'gzip' case. The response stream is managed by pipeline itself, and you should not attempt to modify the response or call res.end() here. Doing so will cause the server to crash.

mateacademy-ai-mentor · 2025-10-26T12:32:50Z

src/createServer.js

+            }
+
+            case 'br': {
+              const deflate = zlib.createBrotliCompress();


This variable holds a Brotli compression stream from zlib.createBrotliCompress(), but it's named deflate. This is a bit confusing. It's better to name it something that reflects its actual type, like brotliStream or compressStream, to make the code easier to understand.

mateacademy-ai-mentor · 2025-10-26T12:32:50Z

src/createServer.js

+              pipeline(readStream, deflate, res, (error) => {
+                res.statusCode = 500;
+                res.end(`Error while piping: ${error}`);
+              });


Similar to the other compression types, this pipeline callback will cause a crash. Please remove the code that modifies the response from this callback, as pipeline already handles stream termination correctly.

solution

3236051

mateacademy-ai-mentor suggested changes Oct 26, 2025

View reviewed changes

solution

cdc4975

Anton-Kuchmasov requested a review from mateacademy-ai-mentor October 26, 2025 12:20

mateacademy-ai-mentor suggested changes Oct 26, 2025

View reviewed changes

solution

f1f8f55

Anton-Kuchmasov requested a review from mateacademy-ai-mentor October 26, 2025 12:31

mateacademy-ai-mentor approved these changes Oct 26, 2025

View reviewed changes


		const compressionType = fields.compressionType[0];

		const uploadedFile = files.filePicker[0];

Conversation

Anton-Kuchmasov commented Oct 26, 2025

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants