Skip to content

feat: implement form data #163

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mykhailokurochkin wants to merge 3 commits into
base: master
Choose a base branch
from
Open

feat: implement form data #163

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6296986
feat: implement form data
mykhailokurochkin Sep 10, 2025
3ed0216
fix: fix createServer issues
mykhailokurochkin Sep 10, 2025
03f3513
refactor: set content-type before piping the form
mykhailokurochkin Sep 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions src/app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,4 @@

const { createServer } = require('./createServer');

createServer().listen(5701, () => {
console.log(`Server is running on http://localhost:${5701} 🚀`);
console.log('Available at http://localhost:5701');
});
createServer().listen(5701);
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a listen callback and/or using an environment variable for the port so the startup is observable and configurable. For example, you could use const PORT = process.env.PORT || 5701; createServer().listen(PORT, () => console.log(Server listening on ${PORT})); — this is optional but helpful for debugging and deployments.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using an environment-configurable port and a listen callback for clarity. For example, use const port = process.env.PORT || 5701 and createServer().listen(port, () => console.log(Listening on ${port})) so the port can be overridden during testing/deployment and you get a startup log.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider making the port configurable and adding a listen callback/logging. For example:

const port = process.env.PORT || 5701;
createServer().listen(port, () => console.log(`Listening on ${port}`));

This is non-blocking — the current hard-coded port works — but it improves flexibility and observability when running in different environments.

199 changes: 197 additions & 2 deletions src/createServer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,203 @@
'use strict';

/* eslint-disable no-console */

const { createReadStream, createWriteStream, mkdirSync } = require('fs');
const http = require('http');
const path = require('path');

function createServer() {
/* Write your code here */
// Return instance of http.Server class
const server = new http.Server();

function getHandler(req, res) {
if (req.method !== 'GET') {
return;
}

if (req.url !== '/' && req.url !== '') {
res.statusCode = 404;
res.setHeader('Content-Type', 'text/plain');
res.end('Not Found');
Comment on lines +18 to +20
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The GET branch returns a 404 with Content-Type 'text/plain' for unknown URLs. It's not required to change this, but for consistency you may want to return an HTML 404 page (text/html; charset=utf-8) so browser users see a friendly page. If you keep it as plain text that's acceptable for the task, but ensure the form page itself is served with text/html.


return;
}

res.statusCode = 200;
res.setHeader('Content-Type', 'text/html; charset=utf-8');
Comment on lines +25 to +26
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good: the GET handler explicitly sets status 200 and Content-Type to text/html; charset=utf-8, ensuring the form is served as HTML. This resolves the earlier blocker about missing Content-Type and makes the form page consistent for browsers and tests.


const form = createReadStream(path.join(__dirname, 'index.html'));
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GET handler streams index.html but does not set Content-Type: text/html before piping. Set the header so the browser renders the content correctly (good practice, not strictly fatal).


form.on('error', (err) => {
console.error('Read stream error:', err);

if (!res.headersSent) {
res.statusCode = 500;
res.setHeader('Content-Type', 'text/html; charset=utf-8');

res.end();
Comment on lines +33 to +37
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The read-stream error handler checks whether headers were already sent, which is good. However, when responding with a 500 it currently calls res.end() with an empty body (lines 33–37). Consider returning a small HTML error page (since GET serves HTML) to make errors clearer to clients and tests. Example: res.end('<!doctype html><html>...<h1>Internal Server Error</h1></html>').

} else {
res.destroy();
}
});

res.on('close', () => {
form.destroy();
});

form.pipe(res);
Comment on lines +28 to +47
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When creating the read stream and piping (createReadStream / form.pipe(res)), the code destroys the read stream when the response closes — good resource handling (lines 28, 43–47).

}

function postHandler(req, res) {
if (req.method !== 'POST') {
return;
}

if (req.url !== '/add-expense' && req.url !== '/submit-expense') {
res.statusCode = 404;
res.setHeader('Content-Type', 'text/plain');
res.end('Not Found');

return;
}

const file = path.resolve('db', 'expense.json');
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code sets const file = path.resolve('db', 'expense.json') but never ensures the db directory exists. If db is missing, create it first (e.g., fs.mkdirSync(path.dirname(file), { recursive: true }) or the async equivalent) before creating a write stream. Without this, writing will fail on a fresh repo.


let raw = '';

req.setEncoding('utf8');
Comment on lines +65 to +67
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no Content-Type inspection before parsing the body. Inspect req.headers['content-type'] and branch parsing logic: JSON.parse only for application/json; use URLSearchParams or querystring.parse for application/x-www-form-urlencoded; do not attempt to parse multipart/form-data unless you implement a parser or remove the enctype on the form. This will make parsing robust.


req.on('data', (chunk) => {
raw += chunk;
});

req.on('end', () => {
const contentType = req.headers['content-type'] || '';
let payload;

try {
if (contentType.includes('application/json')) {
payload = raw ? JSON.parse(raw) : {};
} else if (contentType.includes('application/x-www-form-urlencoded')) {
const params = new URLSearchParams(raw);

payload = Object.fromEntries(params.entries());
} else {
payload = {};
}
} catch (parseErr) {
res.statusCode = 400;
res.setHeader('Content-Type', 'text/plain');
res.end('Bad Request');
Comment on lines +87 to +90
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error branches currently respond with text/plain (e.g., parse error, missing fields, internal server error). Per checklist, return HTML error pages for validation and save failures so the client receives readable messages in the browser. Keep appropriate status codes, but use Content-Type: text/html and a minimal HTML body.


return;
}

const expense = {
date: payload.date,
title: payload.title,
amount: payload.amount,
};

// Validation
const parsedAmount =
typeof expense.amount === 'string'
? parseFloat(expense.amount)
: Number(expense.amount);
const parsedDate = new Date(expense.date);

const hasAll = Boolean(expense.date && expense.title && expense.amount);
const amountValid =
!Number.isNaN(parsedAmount) && Number.isFinite(parsedAmount);
const dateValid = !Number.isNaN(parsedDate.getTime());

if (!hasAll || !amountValid || !dateValid) {
if (contentType.includes('application/json')) {
res.statusCode = 400;
res.setHeader('Content-Type', 'text/plain');
res.end('Invalid payload');
} else {
res.statusCode = 400;
res.setHeader('Content-Type', 'text/html; charset=utf-8');

res.end(
`<!doctype html><html><head><meta charset="utf-8"><title>Error</title></head><body><h1>Invalid input</h1><p>Please provide a valid date, title, and numeric amount.</p></body></html>`,
);
}

return;
}

const normalizedExpense = {
date: expense.date,
title: expense.title,
amount: String(expense.amount),
};

try {
mkdirSync(path.dirname(file), { recursive: true });
Comment on lines +63 to +137
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Path to the data file uses path.resolve('db', 'expense.json'), which matches the requirement to persist to db/expense.json. The directory is created with mkdirSync(path.dirname(file), { recursive: true }) before writing — this satisfies the persistence requirement (lines 63 and 136–137).

} catch (mkdirErr) {
console.error('Directory create error:', mkdirErr);
res.statusCode = 500;
res.setHeader('Content-Type', 'text/plain');
res.end('Internal Server Error');

return;
}

const writer = createWriteStream(file);

writer.on('error', (error) => {
console.error('Write stream error:', error);
res.statusCode = 500;
res.setHeader('Content-Type', 'text/plain');
res.end('Internal Server Error');
Comment on lines +149 to +153
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The writer error handler sets headers and ends the response on write-stream errors. If the response has already begun, setting headers will throw. Consider checking if (!res.headersSent) before setting headers and ending the response, or otherwise handling the case where headers are already sent (for example, destroy the connection). This prevents potential crashes or unhandled exceptions when write errors occur after response start.

});
Comment on lines +149 to +154
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the writer.on('error', ...) handler you set headers and call res.end(). If the write error happens after some of the response was already sent, trying to set headers can fail. Guard with if (!res.headersSent) and otherwise call res.destroy() or log and return. This will avoid errors when the response is already in progress.


const pretty = JSON.stringify(normalizedExpense, null, 2);

writer.end(pretty, () => {
res.statusCode = 200;

if (contentType.includes('application/json')) {
res.setHeader('Content-Type', 'application/json');
res.end(JSON.stringify(normalizedExpense));
} else {
const escaped = pretty
.replace(/&/g, '&amp;')
.replace(/</g, '&lt;')
.replace(/>/g, '&gt;');

res.setHeader('Content-Type', 'text/html; charset=utf-8');

res.end(
`<!doctype html><html><head><meta charset="utf-8"><title>Saved</title></head><body><h1>Expense saved</h1><pre>${escaped}</pre></body></html>`,
Comment on lines +156 to +173
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The saved JSON is pretty-printed (JSON.stringify(normalizedExpense, null, 2)) and escaped before being inserted into an HTML <pre> tag, which satisfies the requirement to return well-formatted JSON in an HTML page after saving. This is implemented at lines 156 and 170–173.

);
}
});
});

req.on('error', (err) => {
console.error('Request error:', err);
res.statusCode = 400;
res.setHeader('Content-Type', 'text/plain');
res.end('Bad Request');
});
}

server.on('request', getHandler);
server.on('request', postHandler);

server.on('request', (req, res) => {
if (req.method !== 'GET' && req.method !== 'POST') {
res.statusCode = 405;
res.setHeader('Content-Type', 'text/plain');
res.end('Method Not Allowed');
}
});

server.on('error', () => {});
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

server.on('error', () => {}) currently swallows server-level errors. At minimum log the error so issues are visible during development (and optionally handle fatal errors). For example:

server.on('error', (err) => {
  console.error('Server error:', err);
});

This will help debugging if the server fails to bind to a port or encounters other system-level errors.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The server currently registers an empty server-level error handler: server.on('error', () => {}); which swallows errors. Replace it with a logger so server errors don't get hidden, e.g. server.on('error', (err) => console.error('Server error:', err)); (line 198).


return server;
}

module.exports = {
Expand Down
28 changes: 28 additions & 0 deletions src/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<!doctype html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>Document</title>
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional improvement: the document <title> is currently Document. Consider making it descriptive (e.g., Add Expense) to help users and accessibility tools identify the page purpose.

</head>
<body>
<form action="/add-expense" method="POST">
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Form action and method are correct: action="/add-expense" method="POST", which matches the server POST handler and will send application/x-www-form-urlencoded data that your server parses. Good.

<label for="date">Date:</label>
<input type="date" id="date" name="date" required /><br /><br />
Comment on lines +10 to +11
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The date input is present with type="date", id="date", name="date", and required. This satisfies the requirement to collect the expense date.


<label for="title">Name:</label>
<input type="text" id="title" name="title" required /><br /><br />
Comment on lines +13 to +14
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor UX suggestion: the label reads Name: but the corresponding input name is title. Consider changing the label to Title: to avoid confusion between field label and internal name. This is not required for functionality but improves clarity.


<label for="amount">Sum:</label>
<input
type="number"
id="amount"
name="amount"
step="0.01"
Comment on lines +17 to +21
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type="date" and type="number" are helpful for client-side validation, but ensure the server validates these values too (e.g., check !isNaN(new Date(date).getTime()) for date and !Number.isNaN(parseFloat(amount)) for amount) before saving to db/expense.json. The task requires server-side validation.

required
Comment on lines +11 to +22
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inputs have the correct name attributes: name="date", name="title", and name="amount". This matches the fields the server should extract. Good. Note: client-side required attributes help UX, but server-side validation of date parseability and numeric amount is still necessary.

/><br /><br />
Comment on lines +16 to +23
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The amount input uses type="number", step="0.01", id="amount", name="amount", and required — this satisfies the requirement to capture a numeric amount and aligns with server-side parsing/validation.


<button type="submit">Submit</button>
</form>
</body>
</html>