Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great job setting up the Node.js server and creating the HTML form! The application correctly serves the form, but there are a few critical issues in how the server handles the POST request that prevent it from meeting the requirements.
Changes are needed in the following areas:
- Data Parsing: Your server attempts to parse the incoming form data with
JSON.parse(), but standard HTML forms submit data in theapplication/x-www-form-urlencodedformat. This causes the server to crash on submission. You should use a parser like Node's built-inURLSearchParamsto handle this data format. - File Overwriting: The current implementation overwrites
expense.jsonwith each new submission, losing all previous entries. To fix this, you need to first read the file's contents, add the new expense to the existing array of expenses, and then write the updated array back to the file. - Response Format: The task requires returning an HTML page that displays the JSON data. Your server is currently sending a raw JSON response (
Content-Type: application/json). Please change theContent-Typetotext/htmland wrap the JSON string in a basic HTML structure, like<pre>tags, to display it correctly in the browser.
Addressing these points will make your application fully functional. Keep up the great work!
✨ 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
| let fields; | ||
|
|
||
| try { | ||
| fields = JSON.parse(body); |
There was a problem hiding this comment.
A standard HTML form submission sends data in the application/x-www-form-urlencoded format, not as a JSON string. As a result, JSON.parse(body) will fail. You should use a parser for URL-encoded data, like Node's built-in URLSearchParams or the querystring module.
src/createServer.js
Outdated
| return server; | ||
| } | ||
|
|
||
| function processExpense(newItem, dbPath, res) { |
There was a problem hiding this comment.
This function overwrites expense.json on every request, which means only the most recent expense will ever be saved. To store a history of expenses, you should first read the contents of expense.json, parse it into an array, add the new expense to the array, and then write the entire updated array back to the file.
src/createServer.js
Outdated
|
|
||
| writeStream.on('finish', () => { | ||
| res.statusCode = 200; | ||
| res.setHeader('Content-Type', 'application/json'); |
There was a problem hiding this comment.
The task requires returning an 'HTML page with well formatted JSON'. This implementation returns a response with the Content-Type of application/json. To meet the requirement, you should set the Content-Type to text/html and wrap the JSON string in basic HTML tags, like <pre>.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great work on this revision! You've successfully addressed most of the previous feedback, and the application is very close to being complete.
However, I'm requesting a change to handle one critical edge case. Currently, the server crashes if db/expense.json doesn't exist on the first run. This happens because the readStream emits an error event that isn't being handled, which prevents the core functionality from working.
To fix this, you can add an error handler to the stream. If the error code is ENOENT (file not found), you can simply proceed with an empty expenses array.
I also have a minor suggestion: for form validation errors, the HTTP status code 400 Bad Request is more semantically correct than 404 Not Found. This isn't a blocker, but good practice to consider.
Once the file creation issue is resolved, this will be ready to go. Keep up the excellent progress!
✨ 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 amount = fields.amount ? fields.amount[0] : null; | ||
|
|
||
| if (!date || !title || !amount) { | ||
| res.statusCode = 404; |
There was a problem hiding this comment.
For form validation errors like this, the HTTP status code 400 Bad Request is generally more appropriate than 404 Not Found. This also applies to the error handling for invalid form data on line 24.
|
|
||
| const dbPath = path.resolve('db', 'expense.json'); | ||
|
|
||
| const readStream = fs.createReadStream(dbPath, { encoding: 'utf8' }); |
There was a problem hiding this comment.
This stream will emit an error event if the db/expense.json file doesn't exist yet, which will crash the server because the error is unhandled. You should add an on('error', ...) handler to the readStream to catch this. If the error code is ENOENT (file not found), you can treat it as an empty list of expenses and proceed.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Excellent work on this revision! I am approving your solution as you've successfully addressed all the previous feedback. The server now handles file system errors gracefully, especially the critical edge case where the database file or directory doesn't exist. Your use of appropriate HTTP status codes for validation errors is also a great improvement. The front-end form is well-structured and submits data correctly. Fantastic job bringing this to completion!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
No description provided.