Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there are a few critical points that will likely cause the task’s tests to fail.
- Filename extension mapping (critical blocker): In
src/createServer.js, you currently derive the output extension directly fromcompressionType(e.g..gzip,.deflate). The requirements specify exact extensions:gzip -> .gz,deflate -> .dfl,br -> .br. Please introduce a small mapping (e.g. an object or switch) that convertscompressionTypeto these extensions before building theContent-Dispositionheader, so the response filename matches the spec. - Field name validation (critical blocker): In the
bb.on('file', fieldname, fileStream, ...)handler insrc/createServer.js, any file field is accepted. The checklist requires the uploaded file field to be named exactlyfile. Add a check likeif (fieldname !== 'file')and either resume/discard that stream and mark the request invalid (respond with 400 onfinish), or immediately send a 400, so malformed forms do not pass as valid. - Unconditional server start (likely test blocker): At the bottom of
src/createServer.js,createServer().listen(3006)runs unconditionally. Test harnesses usually importcreateServerand controllistenthemselves, so this can lead to port conflicts or hanging tests. Wrap thelistencall inif (require.main === module) { ... }or remove it from the module so only the tests or a separate entry file start the server.
Once you fix these points, your streaming logic and error handling structure look solid for this exercise; the changes are focused and will make your implementation align well with the task requirements.
✨ Please rate this review in the Platform's notification center1. 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 button2 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 tag3.
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.
Footnotes
src/createServer.js
Outdated
| } | ||
|
|
||
| // ВИПРАВЛЕННЯ: Використовуємо повне ім'я, як вимагають тести. | ||
| const fileExt = compressionType; |
There was a problem hiding this comment.
The output extension is set directly from compressionType (const fileExt = compressionType;) which will produce names like file.txt.gzip or file.txt.deflate. The task requires specific extensions: gzip -> .gz, deflate -> .dfl, br -> .br. Map compressionType to these exact extensions before constructing the Content-Disposition header.
src/createServer.js
Outdated
| let fileInfo = null; | ||
|
|
||
| // 1. Обробник 'file': ПРИЗУПИНЯЄМО потік і ЗБЕРІГАЄМО логіку | ||
| bb.on('file', (fieldname, fileStream, filename) => { |
There was a problem hiding this comment.
The bb.on('file', ...) handler does not validate the incoming field name. The form input must be named file; verify fieldname === 'file' and treat other file fields as invalid (resume and respond 400 or ignore) so malformed forms are rejected as required by the checklist.
src/createServer.js
Outdated
| return server; | ||
| } | ||
|
|
||
| createServer().listen(3006); |
There was a problem hiding this comment.
The module starts the server unconditionally with createServer().listen(3006). Tests often import createServer and control listening themselves. Consider removing or guarding this automatic listen so the test harness can manage server lifecycle.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there is a critical functional issue that will cause tests to fail and a couple of stream-handling problems that can leave uploads hanging. The main blocker is in src/createServer.js: your EXT_MAP currently maps to compression names like 'gzip', but the requirements expect exact file extensions gzip: 'gz', deflate: 'dfl', br: 'br', so the downloaded files must be named like file.txt.gz, file.txt.dfl, or file.txt.br. In the same file, when an unknown compressionType is detected, you call res.end(...) but never resume or destroy the incoming fileStream; you should explicitly call fileStream.resume() or fileStream.destroy() before ending the response so the upload stream is not left paused. Also in the finish handler there are two consecutive !compressionType checks: the second block (which resumes fileInfo.fileStream) is unreachable, so you should consolidate these into a single branch that both resumes/destroys the stream and returns a 400 with a clear message when compressionType is missing. As a smaller improvement, when setting Content-Disposition, wrap the filename in quotes, e.g. attachment; filename="${filename}.${fileExt}", to correctly handle spaces and special characters once EXT_MAP is fixed. Overall your implementation is very close—once you correct the extension mapping and clean up the stream handling branches, it should fully meet the task requirements.
✨ Please rate this review in the Platform's notification center1. 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 button2 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 tag3.
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.
Footnotes
| const EXT_MAP = { | ||
| gzip: 'gzip', | ||
| deflate: 'deflate', | ||
| br: 'br', | ||
| }; |
There was a problem hiding this comment.
EXT_MAP currently maps to the compression names (e.g. 'gzip') rather than the file extensions the tests require. Update the mapping so the stored extension values are exactly: gzip: 'gz', deflate: 'dfl', br: 'br' so the produced filename becomes file.txt.gz, file.txt.dfl, or file.txt.br as required.
src/createServer.js
Outdated
| res.setHeader( | ||
| 'Content-Disposition', | ||
| `attachment; filename=${filename}.${fileExt}`, | ||
| ); |
There was a problem hiding this comment.
You set the Content-Disposition here. After changing EXT_MAP this will produce the correct extension. Also consider wrapping the filename in quotes to support spaces/special characters, e.g. attachment; filename="${filename}.${fileExt}".
src/createServer.js
Outdated
| if (!EXT_MAP[compressionType]) { | ||
| res.statusCode = 400; | ||
|
|
||
| return res.end('Unknown compression type'); |
There was a problem hiding this comment.
When detecting an unknown compression type you call res.end(...) but the incoming fileStream is still paused/unconsumed. Resume or destroy fileStream before ending the response (e.g. fileStream.resume() or fileStream.destroy()), otherwise the upload stream may be left hanging.
src/createServer.js
Outdated
|
|
||
| if (!compressionType) { | ||
| return res.writeHead(400).end(); | ||
| } | ||
|
|
||
| if (!compressionType) { | ||
| fileInfo.fileStream.resume(); | ||
| res.statusCode = 400; | ||
|
|
||
| return res.end('Missing compressionType field'); |
There was a problem hiding this comment.
There are two consecutive checks for !compressionType in the finish handler (one at lines 110–112 and another at 114–119). The second block (which calls fileInfo.fileStream.resume()) is unreachable — consolidate these into a single finish-branch that resumes/destroys the file stream and returns a 400 with a clear message when compressionType is missing.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution as an educational override so you can keep progressing while you refine a few remaining details.
You’ve done a solid job implementing the streaming server with busboy, zlib, and proper routing/validation for /compress, which covers the core requirements well. There are three specific improvements you should make next:
- In
src/createServer.js, update yourEXT_MAPso it maps to extensions, e.g.gzip: 'gz',deflate: 'dfl',br: 'br', so thatoutNamebecomesfile.txt.gz,file.txt.dfl, orfile.txt.brinstead of using the algorithm names. - Where you handle an unsupported
compressionType(around the!EXT_MAP[compressionType]check), you currently pause the upload stream withfile.pause(); before sending the 400 response, make sure to drain or destroy the file stream (for example, callfile.resume()orfile.destroy()) so the upload doesn’t hang. - When setting
Content-Disposition, wrap the filename in quotes, e.g.attachment; filename="${outName}", so filenames with spaces or special characters are handled correctly.
Overall, your structure and use of streams are on the right track—these refinements will bring your solution fully in line with typical production behavior and the assignment expectations. Keep going; you’re very close to a polished implementation.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| const EXT_MAP = { | ||
| gzip: 'gzip', | ||
| deflate: 'deflate', | ||
| br: 'br', | ||
| }; |
There was a problem hiding this comment.
(HIGH) EXT_MAP currently maps to compression names ('gzip','deflate','br'). The tests expect file extensions so the produced filename must be e.g. file.txt.gz, file.txt.dfl, file.txt.br. Change the mapping to the extension tokens (for example gzip: 'gz', deflate: 'dfl', br: 'br') so outName becomes correct.
| if (!EXT_MAP[compressionType]) { | ||
| res.statusCode = 400; | ||
|
|
||
| return res.end('Invalid compressionType'); |
There was a problem hiding this comment.
(MEDIUM) When !EXT_MAP[compressionType] is detected you set res.statusCode = 400 and return res.end('Invalid compressionType'), but the incoming file stream was paused earlier (file.pause()) and is not resumed or destroyed. This can leave busboy/upload hanging. Resume or destroy the file stream (for example file.resume() or file.destroy()) before ending the response.
| res.setHeader( | ||
| 'Content-Disposition', | ||
| `attachment; filename=${outName}`, |
There was a problem hiding this comment.
(LOW) Wrap the filename in quotes in the Content-Disposition header to handle spaces/special characters correctly. Use e.g. attachment; filename="${outName}" instead of attachment; filename=${outName}.
No description provided.