From 282c4c262d3a19fe3ae3fd9ea75811b816e65dc1 Mon Sep 17 00:00:00 2001 From: Ian Bull Date: Wed, 18 Sep 2024 18:19:45 -0700 Subject: [PATCH] refactor(ext): align error messages (#25496) Aligns the error messages in the ext/http and a few messages in the ext/fetch folder to be in-line with the Deno style guide. This change-set also removes some unnecessary checks in the 00_serve.ts. These options were recently removed, so it doesn't make sense to check for them anymore. https://github.com/denoland/deno/issues/25269 --- ext/fetch/23_request.js | 18 +++++++++--------- ext/http/00_serve.ts | 26 ++++++++++++++------------ ext/http/01_http.js | 6 +++--- tests/unit/serve_test.ts | 10 +++++----- 4 files changed, 31 insertions(+), 29 deletions(-) diff --git a/ext/fetch/23_request.js b/ext/fetch/23_request.js index adebe13b30..6211e927d9 100644 --- a/ext/fetch/23_request.js +++ b/ext/fetch/23_request.js @@ -122,7 +122,7 @@ function newInnerRequest(method, url, headerList, body, maybeBlob) { try { this.headerListInner = headerList(); } catch { - throw new TypeError("cannot read headers: request closed"); + throw new TypeError("Cannot read headers: request closed"); } } return this.headerListInner; @@ -153,7 +153,7 @@ function newInnerRequest(method, url, headerList, body, maybeBlob) { try { this.urlListProcessed[currentIndex] = this.urlList[currentIndex](); } catch { - throw new TypeError("cannot read url: request closed"); + throw new TypeError("Cannot read url: request closed"); } } return this.urlListProcessed[currentIndex]; @@ -193,7 +193,7 @@ function cloneInnerRequest(request, skipBody = false) { try { this.urlListProcessed[0] = this.urlList[0](); } catch { - throw new TypeError("cannot read url: request closed"); + throw new TypeError("Cannot read url: request closed"); } } return this.urlListProcessed[0]; @@ -204,7 +204,7 @@ function cloneInnerRequest(request, skipBody = false) { try { this.urlListProcessed[currentIndex] = this.urlList[currentIndex](); } catch { - throw new TypeError("cannot read url: request closed"); + throw new TypeError("Cannot read url: request closed"); } } return this.urlListProcessed[currentIndex]; @@ -236,13 +236,13 @@ const KNOWN_METHODS = { */ function validateAndNormalizeMethod(m) { if (RegExpPrototypeExec(HTTP_TOKEN_CODE_POINT_RE, m) === null) { - throw new TypeError("Method is not valid."); + throw new TypeError("Method is not valid"); } const upperCase = byteUpperCase(m); if ( upperCase === "CONNECT" || upperCase === "TRACE" || upperCase === "TRACK" ) { - throw new TypeError("Method is forbidden."); + throw new TypeError("Method is forbidden"); } return upperCase; } @@ -418,7 +418,7 @@ class Request { ((init.body !== undefined && init.body !== null) || inputBody !== null) ) { - throw new TypeError("Request with GET/HEAD method cannot have body."); + throw new TypeError("Request with GET/HEAD method cannot have body"); } // 36. @@ -442,7 +442,7 @@ class Request { // 41. if (initBody === null && inputBody !== null) { if (input[_body] && input[_body].unusable()) { - throw new TypeError("Input request's body is unusable."); + throw new TypeError("Input request's body is unusable"); } finalBody = inputBody.createProxy(); } @@ -489,7 +489,7 @@ class Request { const prefix = "Failed to execute 'Request.clone'"; webidl.assertBranded(this, RequestPrototype); if (this[_body] && this[_body].unusable()) { - throw new TypeError("Body is unusable."); + throw new TypeError("Body is unusable"); } const clonedReq = cloneInnerRequest(this[_request]); diff --git a/ext/http/00_serve.ts b/ext/http/00_serve.ts index 6d23685d8e..3b9b085a22 100644 --- a/ext/http/00_serve.ts +++ b/ext/http/00_serve.ts @@ -123,7 +123,7 @@ function upgradeHttpRaw(req, conn) { if (inner._wantsUpgrade) { return inner._wantsUpgrade("upgradeHttpRaw", conn); } - throw new TypeError("upgradeHttpRaw may only be used with Deno.serve"); + throw new TypeError("'upgradeHttpRaw' may only be used with Deno.serve"); } function addTrailers(resp, headerList) { @@ -170,10 +170,10 @@ class InnerRequest { _wantsUpgrade(upgradeType, ...originalArgs) { if (this.#upgraded) { - throw new Deno.errors.Http("already upgraded"); + throw new Deno.errors.Http("Already upgraded"); } if (this.#external === null) { - throw new Deno.errors.Http("already closed"); + throw new Deno.errors.Http("Already closed"); } // upgradeHttpRaw is sync @@ -257,7 +257,7 @@ class InnerRequest { if (this.#methodAndUri === undefined) { if (this.#external === null) { - throw new TypeError("request closed"); + throw new TypeError("Request closed"); } // TODO(mmastrac): This is quite slow as we're serializing a large number of values. We may want to consider // splitting this up into multiple ops. @@ -315,7 +315,7 @@ class InnerRequest { } if (this.#methodAndUri === undefined) { if (this.#external === null) { - throw new TypeError("request closed"); + throw new TypeError("Request closed"); } this.#methodAndUri = op_http_get_request_method_and_url(this.#external); } @@ -329,7 +329,7 @@ class InnerRequest { get method() { if (this.#methodAndUri === undefined) { if (this.#external === null) { - throw new TypeError("request closed"); + throw new TypeError("Request closed"); } this.#methodAndUri = op_http_get_request_method_and_url(this.#external); } @@ -338,7 +338,7 @@ class InnerRequest { get body() { if (this.#external === null) { - throw new TypeError("request closed"); + throw new TypeError("Request closed"); } if (this.#body !== undefined) { return this.#body; @@ -356,7 +356,7 @@ class InnerRequest { get headerList() { if (this.#external === null) { - throw new TypeError("request closed"); + throw new TypeError("Request closed"); } const headers = []; const reqHeaders = op_http_get_request_headers(this.#external); @@ -457,7 +457,7 @@ function fastSyncResponseOrStream( // At this point in the response it needs to be a stream if (!ObjectPrototypeIsPrototypeOf(ReadableStreamPrototype, stream)) { innerRequest?.close(); - throw new TypeError("invalid response"); + throw new TypeError("Invalid response"); } const resourceBacking = getReadableStreamResourceBacking(stream); let rid, autoClose; @@ -619,13 +619,15 @@ function serve(arg1, arg2) { if (handler === undefined) { if (options === undefined) { throw new TypeError( - "No handler was provided, so an options bag is mandatory.", + "Cannot serve HTTP requests: either a `handler` or `options` must be specified", ); } handler = options.handler; } if (typeof handler !== "function") { - throw new TypeError("A handler function must be provided."); + throw new TypeError( + `Cannot serve HTTP requests: handler must be a function, received ${typeof handler}`, + ); } if (options === undefined) { options = { __proto__: null }; @@ -679,7 +681,7 @@ function serve(arg1, arg2) { if (wantsHttps) { if (!options.cert || !options.key) { throw new TypeError( - "Both cert and key must be provided to enable HTTPS.", + "Both 'cert' and 'key' must be provided to enable HTTPS", ); } listenOpts.cert = options.cert; diff --git a/ext/http/01_http.js b/ext/http/01_http.js index e886848581..9302bd8a0f 100644 --- a/ext/http/01_http.js +++ b/ext/http/01_http.js @@ -207,7 +207,7 @@ function createRespondWith( resp = await resp; if (!(ObjectPrototypeIsPrototypeOf(ResponsePrototype, resp))) { throw new TypeError( - "First argument to respondWith must be a Response or a promise resolving to a Response.", + "First argument to 'respondWith' must be a Response or a promise resolving to a Response", ); } @@ -220,7 +220,7 @@ function createRespondWith( let respBody = null; if (innerResp.body !== null) { if (innerResp.body.unusable()) { - throw new TypeError("Body is unusable."); + throw new TypeError("Body is unusable"); } if ( ObjectPrototypeIsPrototypeOf( @@ -295,7 +295,7 @@ function createRespondWith( let reader; if (resourceBacking) { if (respBody.locked) { - throw new TypeError("ReadableStream is locked."); + throw new TypeError("ReadableStream is locked"); } reader = respBody.getReader(); // Acquire JS lock. try { diff --git a/tests/unit/serve_test.ts b/tests/unit/serve_test.ts index d417d8d268..439d71d553 100644 --- a/tests/unit/serve_test.ts +++ b/tests/unit/serve_test.ts @@ -1592,7 +1592,7 @@ Deno.test( Deno.upgradeWebSocket(request); }, Deno.errors.Http, - "already upgraded", + "Already upgraded", ); socket.onerror = (e) => { console.error(e); @@ -3694,7 +3694,7 @@ Deno.test( } catch (cloneError) { assert(cloneError instanceof TypeError); assert( - cloneError.message.endsWith("Body is unusable."), + cloneError.message.endsWith("Body is unusable"), ); ac.abort(); @@ -3743,7 +3743,7 @@ Deno.test({ } catch (cloneError) { assert(cloneError instanceof TypeError); assert( - cloneError.message.endsWith("Body is unusable."), + cloneError.message.endsWith("Body is unusable"), ); ac.abort(); @@ -3895,7 +3895,7 @@ async function readTrailers( const tp = new TextProtoReader(r); const result = await tp.readMimeHeader(); if (result == null) { - throw new Deno.errors.InvalidData("Missing trailer header."); + throw new Deno.errors.InvalidData("Missing trailer header"); } const undeclared = [...result.keys()].filter( (k) => !trailerNames.includes(k), @@ -3923,7 +3923,7 @@ function parseTrailer(field: string | null): Headers | undefined { } const trailerNames = field.split(",").map((v) => v.trim().toLowerCase()); if (trailerNames.length === 0) { - throw new Deno.errors.InvalidData("Empty trailer header."); + throw new Deno.errors.InvalidData("Empty trailer header"); } const prohibited = trailerNames.filter((k) => isProhibitedForTrailer(k)); if (prohibited.length > 0) {