Skip to content
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.

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

refactor(ext/fetch): align error messages #25374

Merged
merged 5 commits into from
Sep 4, 2024

Conversation

irbull
Copy link
Contributor

@irbull irbull commented Sep 3, 2024

Aligns the error messages in the ext/fetch folder to be in-line with the Deno style guide.

#25269

Aligns the error messages in the ext/fetch folder to be in-line with the
Deno style guide.

denoland#25269
ext/fetch/20_headers.js Outdated Show resolved Hide resolved
ext/fetch/20_headers.js Outdated Show resolved Hide resolved
@@ -396,7 +396,7 @@ class MultipartParser {
*/
constructor(body, boundary) {
if (!boundary) {
throw new TypeError("multipart/form-data must provide a boundary");
throw new TypeError("Cannot construct MultipartParser: multipart/form-data must provide a boundary");
Copy link
Member

Choose a reason for hiding this comment

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

This is not a public API - so this doesn't matter. But it's ok :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

While some of these are not public API, do any of these exceptions propagate high enough that the user would see them?

ext/fetch/22_body.js Outdated Show resolved Hide resolved
ext/fetch/26_fetch.js Outdated Show resolved Hide resolved
Copy link
Contributor

@marvinhagemeister marvinhagemeister left a comment

Choose a reason for hiding this comment

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

LGTM, left two nitpicky comments. Let me know what you think

ext/fetch/23_response.js Outdated Show resolved Hide resolved
ext/fetch/23_response.js Outdated Show resolved Hide resolved
Co-authored-by: Marvin Hagemeister <[email protected]>
Signed-off-by: Ian Bull <[email protected]>
@marvinhagemeister marvinhagemeister merged commit ce6b675 into denoland:main Sep 4, 2024
17 checks passed
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.

3 participants