Refactor fetch.ts to improve error handling and formatting #628
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -74,7 +74,7 @@ export function traceFetchPost( | |||
| ${Object.entries(headers) | |||
| .map(([k, v]) => `-H "${k}: ${v}" \\`) | |||
| .join("\n")} | |||
| -d '${JSON.stringify(body).replace(/'/g, "'\\''").replace(/\r\n/g, "\n \\")}' | |||
| -d '${JSON.stringify(body, null, 2).replace(/'/g, "'\\''").replace(/\r?\n/g, "\n \\")}' | |||
There was a problem hiding this comment.
The use of JSON.stringify(body, null, 2) will format the JSON with 2 spaces of indentation. This might not be the desired behavior if the intention was to send the JSON data as compact as possible to reduce network usage. Consider reverting this change if compactness is a priority. 📡
generated by pr-review-commit
JSON_stringify
| @@ -74,7 +74,7 @@ export function traceFetchPost( | |||
| ${Object.entries(headers) | |||
| .map(([k, v]) => `-H "${k}: ${v}" \\`) | |||
| .join("\n")} | |||
| -d '${JSON.stringify(body).replace(/'/g, "'\\''").replace(/\r\n/g, "\n \\")}' | |||
| -d '${JSON.stringify(body, null, 2).replace(/'/g, "'\\''").replace(/\r?\n/g, "\n \\")}' | |||
There was a problem hiding this comment.
The replacement of \r\n with \n \\ has been changed to replace \r?\n with \n \\. This change will also replace single \n characters with \n \\, which might not be the desired behavior if the intention was to only replace Windows-style line endings (\r\n). Please confirm if this change is intentional. 🖥️
generated by pr-review-commit
replace
| @@ -38,7 +38,7 @@ export async function createFetch( | |||
| if (code === "ECONNRESET" || code === "ENOTFOUND") | |||
| // fatal | |||
| return undefined | |||
|
|
|||
There was a problem hiding this comment.
The removal of the whitespace on line 41 does not seem to have any functional impact, but it might affect code readability if the whitespace was intended to separate logical blocks of code. Please confirm if this change is intentional. 📖
generated by pr-review-commit
whitespace
|
The changes in the pull request are relatively minor and focused on code readability and format.
Overall, there don't seem to be any functional concerns with these changes. So, LGTM 🚀
|
This pull request refactors the
fetch.tsfile to improve error handling and formatting. The changes include handling specific error codes, checking for cancellation, and improving the formatting of the request body in thetraceFetchPostfunction. These improvements enhance the overall reliability and readability of the code.The GIT_DIFF shows two changes made in the
fetch.tsfile present in thepackages/core/src/directory:🧹Empty space was removed on line 38 and 74, making the code more concise and clean, following best coding practices.
📝On line 74, the way the body of the fetch request is stringified for debugging purposes was modified. The changes effectively pretty-print the JSON body (adds indentation) and ensure newlines are consistently replaced, regardless of whether they're part of a Windows-style or Unix-style line-ending.
Please note, these alterations do not have an impact on the public API and would likely be considered internal or housekeeping changes.