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
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
22 changes: 11 additions & 11 deletions ext/fetch/20_headers.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ function fillHeaders(headers, object) {
const header = object[i];
if (header.length !== 2) {
throw new TypeError(
`Invalid header. Length must be 2, but is ${header.length}`,
`Invalid header: length must be 2, but is ${header.length}`,
);
}
appendHeader(headers, header[0], header[1]);
Expand Down Expand Up @@ -133,15 +133,15 @@ function appendHeader(headers, name, value) {

// 2.
if (!checkHeaderNameForHttpTokenCodePoint(name)) {
throw new TypeError("Header name is not valid.");
throw new TypeError(`Invalid header name: "${name}"`);
}
if (!checkForInvalidValueChars(value)) {
throw new TypeError("Header value is not valid.");
throw new TypeError(`Invalid header value: "${value}"`);
}

// 3.
if (headers[_guard] == "immutable") {
throw new TypeError("Headers are immutable.");
throw new TypeError("Cannot change header: headers are immutable");
}

// 7.
Expand Down Expand Up @@ -330,10 +330,10 @@ class Headers {
name = webidl.converters["ByteString"](name, prefix, "Argument 1");

if (!checkHeaderNameForHttpTokenCodePoint(name)) {
throw new TypeError("Header name is not valid.");
throw new TypeError(`Invalid header name: "${name}"`);
}
if (this[_guard] == "immutable") {
throw new TypeError("Headers are immutable.");
throw new TypeError("Cannot change header: headers are immutable");
irbull marked this conversation as resolved.
Show resolved Hide resolved
}

const list = this[_headerList];
Expand All @@ -356,7 +356,7 @@ class Headers {
name = webidl.converters["ByteString"](name, prefix, "Argument 1");

if (!checkHeaderNameForHttpTokenCodePoint(name)) {
throw new TypeError("Header name is not valid.");
throw new TypeError(`Invalid header name: "${name}"`);
}

const list = this[_headerList];
Expand Down Expand Up @@ -387,7 +387,7 @@ class Headers {
name = webidl.converters["ByteString"](name, prefix, "Argument 1");

if (!checkHeaderNameForHttpTokenCodePoint(name)) {
throw new TypeError("Header name is not valid.");
throw new TypeError(`Invalid header name: "${name}"`);
}

const list = this[_headerList];
Expand Down Expand Up @@ -415,14 +415,14 @@ class Headers {

// 2.
if (!checkHeaderNameForHttpTokenCodePoint(name)) {
throw new TypeError("Header name is not valid.");
throw new TypeError(`Invalid header name: "${name}"`);
}
if (!checkForInvalidValueChars(value)) {
throw new TypeError("Header value is not valid.");
throw new TypeError(`Invalid header value: "${value}"`);
}

if (this[_guard] == "immutable") {
throw new TypeError("Headers are immutable.");
throw new TypeError("Cannot change header: headers are immutable");
irbull marked this conversation as resolved.
Show resolved Hide resolved
}

const list = this[_headerList];
Expand Down
4 changes: 2 additions & 2 deletions ext/fetch/21_formdata.js
Original file line number Diff line number Diff line change
Expand Up @@ -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?

}

this.boundary = `--${boundary}`;
Expand Down Expand Up @@ -445,7 +445,7 @@ class MultipartParser {
) {
return new FormData();
}
throw new TypeError("Unable to parse body as form data.");
throw new TypeError("Unable to parse body as form data");
}

const formData = new FormData();
Expand Down
4 changes: 2 additions & 2 deletions ext/fetch/22_body.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ class InnerBody {
* @returns {Promise<Uint8Array>}
*/
consume() {
if (this.unusable()) throw new TypeError("Body already consumed.");
if (this.unusable()) throw new TypeError("Body already consumed");
if (
ObjectPrototypeIsPrototypeOf(
ReadableStreamPrototype,
Expand Down Expand Up @@ -372,7 +372,7 @@ function packageData(bytes, type, mimeType) {
const boundary = mimeType.parameters.get("boundary");
if (boundary === null) {
throw new TypeError(
"Missing boundary parameter in mime type of multipart formdata.",
"Cannot package data: missing boundary parameter in mime type of multipart formdata",
irbull marked this conversation as resolved.
Show resolved Hide resolved
);
}
return parseFormData(chunkToU8(bytes), boundary);
Expand Down
8 changes: 4 additions & 4 deletions ext/fetch/23_response.js
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ function initializeAResponse(response, init, bodyWithType) {
// 1.
if ((init.status < 200 || init.status > 599) && init.status != 101) {
throw new RangeError(
`The status provided (${init.status}) is not equal to 101 and outside the range [200, 599].`,
`The status provided (${init.status}) is not equal to 101 and outside the range [200, 599]`,
);
}

Expand All @@ -181,7 +181,7 @@ function initializeAResponse(response, init, bodyWithType) {
init.statusText &&
RegExpPrototypeExec(REASON_PHRASE_RE, init.statusText) === null
) {
throw new TypeError("Status text is not valid.");
throw new TypeError(`Status text is not valid: received "${init.statusText}"`);
}

// 3.
Expand Down Expand Up @@ -263,7 +263,7 @@ class Response {
const baseURL = getLocationHref();
const parsedURL = new URL(url, baseURL);
if (!redirectStatus(status)) {
throw new RangeError("Invalid redirect status code.");
throw new RangeError(`Invalid redirect status code: received ${status}`);
irbull marked this conversation as resolved.
Show resolved Hide resolved
}
const inner = newInnerResponse(status);
inner.type = "default";
Expand Down Expand Up @@ -395,7 +395,7 @@ class Response {
clone() {
webidl.assertBranded(this, ResponsePrototype);
if (this[_body] && this[_body].unusable()) {
throw new TypeError("Body is unusable.");
throw new TypeError("Body is unusable");
}
const second = webidl.createBranded(Response);
const newRes = cloneInnerResponse(this[_response]);
Expand Down
8 changes: 4 additions & 4 deletions ext/fetch/26_fetch.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ function createResponseBodyStream(responseBodyRid, terminator) {
async function mainFetch(req, recursive, terminator) {
if (req.blobUrlEntry !== null) {
if (req.method !== "GET") {
throw new TypeError("Blob URL fetch only supports GET method.");
throw new TypeError("Blob URL fetch only supports GET method");
}

const body = new InnerBody(req.blobUrlEntry.stream());
Expand Down Expand Up @@ -145,7 +145,7 @@ async function mainFetch(req, recursive, terminator) {
reqRid = resourceForReadableStream(stream, req.body.length);
}
} else {
throw new TypeError("invalid body");
throw new TypeError("Invalid body");
}
}

Expand Down Expand Up @@ -441,13 +441,13 @@ function handleWasmStreaming(source, rid) {
typeof contentType !== "string" ||
StringPrototypeToLowerCase(contentType) !== "application/wasm"
) {
throw new TypeError("Invalid WebAssembly content type.");
throw new TypeError("Invalid WebAssembly content type");
}
}

// 2.5.
if (!res.ok) {
throw new TypeError(`HTTP status code ${res.status}`);
throw new TypeError(`Cannot handle Wasm streaming: HTTP status code ${res.status}`);
irbull marked this conversation as resolved.
Show resolved Hide resolved
}

// Pass the resolved URL to v8.
Expand Down
2 changes: 1 addition & 1 deletion ext/fetch/fs_fetch_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ impl FetchHandler for FsFetchHandler {
Ok::<_, ()>(response)
}
.map_err(move |_| {
type_error("NetworkError when attempting to fetch resource.")
type_error("NetworkError when attempting to fetch resource")
})
.or_cancel(&cancel_handle)
.boxed_local();
Expand Down
16 changes: 8 additions & 8 deletions ext/fetch/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ impl FetchHandler for DefaultFileFetchHandler {
) -> (CancelableResponseFuture, Option<Rc<CancelHandle>>) {
let fut = async move {
Ok(Err(type_error(
"NetworkError when attempting to fetch resource.",
"NetworkError when attempting to fetch resource",
)))
};
(Box::pin(fut), None)
Expand Down Expand Up @@ -361,14 +361,14 @@ where
let (request_rid, cancel_handle_rid) = match scheme {
"file" => {
let path = url.to_file_path().map_err(|_| {
type_error("NetworkError when attempting to fetch resource.")
type_error("NetworkError when attempting to fetch resource")
})?;
let permissions = state.borrow_mut::<FP>();
permissions.check_read(&path, "fetch()")?;

if method != Method::GET {
return Err(type_error(format!(
"Fetching files only supports the GET method. Received {method}."
"Fetching files only supports the GET method: received {method}"
)));
}

Expand All @@ -394,7 +394,7 @@ where
let uri = url
.as_str()
.parse::<Uri>()
.map_err(|_| type_error("Invalid URL"))?;
.map_err(|_| type_error(format!("Invalid URL {url}")))?;

let mut con_len = None;
let body = if has_body {
Expand Down Expand Up @@ -522,7 +522,7 @@ where
// because the URL isn't an object URL.
return Err(type_error("Blob for the given URL not found."));
}
_ => return Err(type_error(format!("scheme '{scheme}' not supported"))),
_ => return Err(type_error(format!("Url scheme '{scheme}' not supported"))),
};

Ok(FetchReturn {
Expand Down Expand Up @@ -586,7 +586,7 @@ pub async fn op_fetch_send(

return Err(type_error(err.to_string()));
}
Err(_) => return Err(type_error("request was cancelled")),
Err(_) => return Err(type_error("Request was cancelled")),
};

let status = res.status();
Expand Down Expand Up @@ -1018,7 +1018,7 @@ pub fn create_http_client(

let user_agent = user_agent
.parse::<HeaderValue>()
.map_err(|_| type_error("illegal characters in User-Agent"))?;
.map_err(|_| type_error(format!("Illegal characters in User-Agent: received {user_agent}")))?;

let mut builder =
hyper_util::client::legacy::Builder::new(TokioExecutor::new());
Expand Down Expand Up @@ -1060,7 +1060,7 @@ pub fn create_http_client(
}
(true, true) => {}
(false, false) => {
return Err(type_error("Either `http1` or `http2` needs to be true"))
return Err(type_error("Cannot create Http Client: either `http1` or `http2` needs to be set to true"))
}
}

Expand Down
2 changes: 1 addition & 1 deletion tests/testdata/run/wasm_streaming_panic_test.js.out
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
error: Uncaught (in promise) TypeError: Invalid WebAssembly content type.
error: Uncaught (in promise) TypeError: Invalid WebAssembly content type
at handleWasmStreaming (ext:deno_fetch/26_fetch.js:[WILDCARD])
2 changes: 1 addition & 1 deletion tests/unit/cache_api_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ Deno.test(async function cachePutReaderLock() {
await response.arrayBuffer();
},
TypeError,
"Body already consumed.",
"Body already consumed",
);

await promise;
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/fetch_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1131,7 +1131,7 @@ Deno.test(function fetchResponseConstructorInvalidStatus() {
assert(e instanceof RangeError);
assert(
e.message.endsWith(
"is not equal to 101 and outside the range [200, 599].",
"is not equal to 101 and outside the range [200, 599]",
),
);
}
Expand Down Expand Up @@ -1662,7 +1662,7 @@ Deno.test(
);
},
TypeError,
"Fetching files only supports the GET method. Received POST.",
"Fetching files only supports the GET method: received POST",
);
},
);
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/headers_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -406,11 +406,11 @@ Deno.test(function invalidHeadersFlaky() {
assertThrows(
() => new Headers([["x", "\u0000x"]]),
TypeError,
"Header value is not valid.",
"Invalid header value: \"\u0000x\"",
);
assertThrows(
() => new Headers([["x", "\u0000x"]]),
TypeError,
"Header value is not valid.",
"Invalid header value: \"\u0000x\"",
);
});
2 changes: 1 addition & 1 deletion tests/unit/wasm_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ Deno.test(
await assertRejects(
() => wasmPromise,
TypeError,
"Invalid WebAssembly content type.",
"Invalid WebAssembly content type",
);
},
);
Expand Down
Loading