Skip to content

Solution#426

Open
IraPolchanin wants to merge 2 commits intomate-academy:masterfrom
IraPolchanin:develop
Open

Solution#426
IraPolchanin wants to merge 2 commits intomate-academy:masterfrom
IraPolchanin:develop

Conversation

@IraPolchanin
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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

Review Summary

Decision: REQUEST_CHANGES

Your server meets most requirements, but there is one critical blocker: path segments are not URL-decoded, which violates requirement 3.6 and can cause incorrect output for percent-encoded paths.

Critical issues to fix:

  • Requirement 3.6: Decode each non-empty path segment. In src/createServer.js, where you do const parts = parsedUrl.pathname.split('/')..., add .map with decodeURIComponent and guard against URIError so malformed encodings don’t crash the server. For example:
    • Split, filter non-empty, then map with a try/catch: try { decodeURIComponent(seg) } catch { /* fallback: keep raw seg or respond 400 */ }.
    • This ensures inputs like /a%20b yield parts: ["a b"].

Minor suggestions (non-blockers):

  • Set res.statusCode = 200 explicitly for clarity (req. 3.5 optional enhancement).
  • Ensure malformed percent-encodings are handled gracefully before JSON.stringify so the server never crashes (requirements 2.6/3.6). Optionally, consider falling back to the raw segment or returning 400.

Overall, great job on query parsing, empty-segment filtering, JSON response structure, and headers. Once you add decoding (with safe error handling), the implementation should fully satisfy the checklist. Keep it up!


✨ 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

// Return instance of http.Server class
const server = http.createServer((req, res) => {
const parsedUrl = new URL(req.url, 'http://localhost');
const pathName = parsedUrl.pathname;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The pathname is read here as parsedUrl.pathname. Path segments extracted later are not decoded; percent-encoded characters (e.g. %20) will remain encoded. Requirement 3.6 requires URL-decoding of path segments so output reflects human-readable strings. Consider decoding each segment with decodeURIComponent and guard against malformed encodings (URIError).

const server = http.createServer((req, res) => {
const parsedUrl = new URL(req.url, 'http://localhost');
const pathName = parsedUrl.pathname;
const query = Object.fromEntries(parsedUrl.searchParams);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good: converting parsedUrl.searchParams into an object via Object.fromEntries yields an object of string values and returns decoded keys/values. This satisfies the requirement that query parameters be parsed into an object of strings (requirement 3.2).

const parsedUrl = new URL(req.url, 'http://localhost');
const pathName = parsedUrl.pathname;
const query = Object.fromEntries(parsedUrl.searchParams);
const parts = pathName.split('/').filter((part) => part !== '');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The split + filter here removes empty segments (doubled or leading/trailing slashes), which is correct for parts extraction. However, add a .map to decode each filtered segment (with try/catch for decodeURIComponent) to meet the URL-decoding requirement (3.6).

const query = Object.fromEntries(parsedUrl.searchParams);
const parts = pathName.split('/').filter((part) => part !== '');

res.setHeader('Content-Type', 'application/json');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You set the Content-Type header to application/json which satisfies requirement 2.5. Optionally, set res.statusCode = 200 explicitly to make the successful response code explicit (requirement 3.5).

query,
};

res.end(JSON.stringify(result));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Final JSON stringification is correct. If you implement decoding you can still JSON.stringify the resulting object. Ensure to handle any decode errors before calling JSON.stringify so the server doesn't crash on malformed percent-encoding (requirement 2.6 and 3.6).

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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

Review Summary

The server meets the core requirements: it parses the pathname into non-empty parts, safely decodes segments with decodeURIComponent inside a try/catch (preventing crashes on malformed encodings), parses the query into a string-valued object via Object.fromEntries(parsedUrl.searchParams), and returns the exact JSON shape with explicit 200 status and JSON Content-Type. Nice job closing the earlier critical gap around decoding.

Minor, non-blocking suggestions:

  • Consider making the Content-Type explicit about encoding: application/json; charset=utf-8 (e.g., at src/createServer.js, where you set headers).
  • Decide on policy for malformed encodings: keeping the raw segment (current behavior) is acceptable; alternatively, you could respond with 400. If you keep the fallback, add a brief comment to document it.
  • Be aware that repeated query keys keep the last value (e.g., ?a=1&a=2 => a: "2"); this is fine for the task.

Overall, the implementation is correct and robust for the task. Well done.


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

Footnotes

  1. Rate AI review example

Comment on lines +11 to +19
const parts = pathName
.split('/')
.filter((part) => part !== '')
.map((segment) => {
try {
return decodeURIComponent(segment);
} catch (error) {
return segment;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good: the code decodes each non-empty path segment with decodeURIComponent inside a try/catch, so malformed percent-encodings won't crash the server and the requirement to decode path segments safely is satisfied. This addresses the previous critical blocker about decoding. Consider whether you want to fall back to the raw segment (current behavior) or respond with a 400 for malformed encodings — both are acceptable choices depending on desired strictness.

const server = http.createServer((req, res) => {
const parsedUrl = new URL(req.url, 'http://localhost');
const pathName = parsedUrl.pathname;
const query = Object.fromEntries(parsedUrl.searchParams);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Query parsing using Object.fromEntries(parsedUrl.searchParams) is correct and yields an object with string values (and an empty object when no query params are present). Note: if a query key appears multiple times (e.g. ?a=1&a=2), this approach will keep the last value ("2"). The task doesn't require special handling for repeated keys, but be aware of this behavior.

}
});
res.statusCode = 200;
res.setHeader('Content-Type', 'application/json');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Minor suggestion: make the response Content-Type explicit about character encoding, e.g. res.setHeader('Content-Type', 'application/json; charset=utf-8'). This is optional and does not affect correctness for the task.

return segment;
}
});
res.statusCode = 200;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Small confirmation: setting res.statusCode = 200 explicitly is good for clarity and ensures clients receive an explicit success status. Keep this as-is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants