From b7e6a31a425901c089f4b524774b985906982fae Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Thu, 19 Mar 2020 23:15:21 +0000 Subject: [PATCH] fix(std/http): Fix respond error test on Windows (#4408) --- std/http/io.ts | 2 +- std/http/server_test.ts | 57 ++++++++--------------------------------- std/testing/asserts.ts | 6 ++--- 3 files changed, 15 insertions(+), 50 deletions(-) diff --git a/std/http/io.ts b/std/http/io.ts index b53b525277..5518146a80 100644 --- a/std/http/io.ts +++ b/std/http/io.ts @@ -242,7 +242,7 @@ export async function writeResponse( const statusText = STATUS_TEXT.get(statusCode); const writer = BufWriter.create(w); if (!statusText) { - throw Error("bad status code"); + throw new Deno.errors.InvalidData("Bad status code"); } if (!r.body) { r.body = new Uint8Array(); diff --git a/std/http/server_test.ts b/std/http/server_test.ts index d3f36a1d34..2db44f260f 100644 --- a/std/http/server_test.ts +++ b/std/http/server_test.ts @@ -10,11 +10,12 @@ import { assert, assertEquals, assertNotEOF, - assertStrContains + assertStrContains, + assertThrowsAsync } from "../testing/asserts.ts"; import { Response, ServerRequest, Server, serve } from "./server.ts"; import { BufReader, BufWriter } from "../io/bufio.ts"; -import { delay, deferred } from "../util/async.ts"; +import { delay } from "../util/async.ts"; import { encode, decode } from "../strings/mod.ts"; import { mockConn } from "./mock.ts"; @@ -488,57 +489,23 @@ test({ } }); -// TODO(kevinkassimo): create a test that works on Windows. -// The following test is to ensure that if an error occurs during respond -// would result in connection closed. (such that fd/resource is freed). -// On *nix, a delayed second attempt to write to a CLOSE_WAIT connection would -// receive a RST and thus trigger an error during response for us to test. -// We need to find a way to similarly trigger an error on Windows so that -// we can test if connection is closed. test({ - ignore: Deno.build.os == "win", - name: "respond error handling", + name: "respond error closes connection", async fn(): Promise { - const connClosedPromise = deferred(); const serverRoutine = async (): Promise => { - let reqCount = 0; const server = serve(":8124"); // @ts-ignore - const serverRid = server.listener["rid"]; - let connRid = -1; for await (const req of server) { - connRid = req.conn.rid; - reqCount++; - await Deno.readAll(req.body); - await connClosedPromise; - try { + await assertThrowsAsync(async () => { await req.respond({ + status: 12345, body: new TextEncoder().encode("Hello World") }); - await delay(100); - req.done = deferred(); - // This duplicate respond is to ensure we get a write failure from the - // other side. Our client would enter CLOSE_WAIT stage after close(), - // meaning first server .send (.respond) after close would still work. - // However, a second send would fail under RST, which is similar - // to the scenario where a failure happens during .respond - await req.respond({ - body: new TextEncoder().encode("Hello World") - }); - } catch { - break; - } + }, Deno.errors.InvalidData); + // The connection should be destroyed + assert(!(req.conn.rid in Deno.resources())); + server.close(); } - server.close(); - // Let event loop do another turn so server - // finishes all pending ops. - await delay(0); - const resources = Deno.resources(); - assert(reqCount === 1); - // Server should be gone - assert(!(serverRid in resources)); - // The connection should be destroyed - assert(!(connRid in resources)); }; const p = serverRoutine(); const conn = await Deno.connect({ @@ -549,9 +516,7 @@ test({ conn, new TextEncoder().encode("GET / HTTP/1.1\r\n\r\n") ); - conn.close(); // abruptly closing connection before response. - // conn on server side enters CLOSE_WAIT state. - connClosedPromise.resolve(); + conn.close(); await p; } }); diff --git a/std/testing/asserts.ts b/std/testing/asserts.ts index 1c6dfff19c..44c3112043 100644 --- a/std/testing/asserts.ts +++ b/std/testing/asserts.ts @@ -348,9 +348,9 @@ export async function assertThrowsAsync( await fn(); } catch (e) { if (ErrorClass && !(Object.getPrototypeOf(e) === ErrorClass.prototype)) { - msg = `Expected error to be instance of "${ErrorClass.name}"${ - msg ? `: ${msg}` : "." - }`; + msg = `Expected error to be instance of "${ErrorClass.name}", but got "${ + e.name + }"${msg ? `: ${msg}` : "."}`; throw new AssertionError(msg); } if (msgIncludes && !e.message.includes(msgIncludes)) {