From 12c48f91174edc78a0eaf91c32b5d2c7c9942b39 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Thu, 7 Nov 2024 17:12:13 +0530 Subject: [PATCH] feat(ext/http): abort signal when request is cancelled (#26761) Closes https://github.com/denoland/deno/issues/21653 --- ext/fetch/23_request.js | 16 +++++++++++----- ext/http/00_serve.ts | 8 ++++++++ ext/http/http_next.rs | 8 ++++++++ ext/http/lib.rs | 1 + tests/unit/serve_test.ts | 29 +++++++++++++++++++++++++++++ 5 files changed, 57 insertions(+), 5 deletions(-) diff --git a/ext/fetch/23_request.js b/ext/fetch/23_request.js index 6211e927d92d1d..22c17d6d23b841 100644 --- a/ext/fetch/23_request.js +++ b/ext/fetch/23_request.js @@ -269,19 +269,25 @@ class Request { /** @type {AbortSignal} */ get [_signal]() { const signal = this[_signalCache]; - // This signal not been created yet, and the request is still in progress - if (signal === undefined) { + // This signal has not been created yet, but the request has already completed + if (signal === false) { const signal = newSignal(); this[_signalCache] = signal; + signal[signalAbort](signalAbortError); return signal; } - // This signal has not been created yet, but the request has already completed - if (signal === false) { + + // This signal not been created yet, and the request is still in progress + if (signal === undefined) { const signal = newSignal(); this[_signalCache] = signal; - signal[signalAbort](signalAbortError); return signal; } + + if (!signal.aborted && this[_request].isCancelled) { + signal[signalAbort](signalAbortError); + } + return signal; } get [_mimeType]() { diff --git a/ext/http/00_serve.ts b/ext/http/00_serve.ts index 1b70cf21291cc6..8cfd7ad535e58b 100644 --- a/ext/http/00_serve.ts +++ b/ext/http/00_serve.ts @@ -11,6 +11,7 @@ import { op_http_cancel, op_http_close, op_http_close_after_finish, + op_http_get_request_cancelled, op_http_get_request_headers, op_http_get_request_method_and_url, op_http_read_request_body, @@ -373,6 +374,13 @@ class InnerRequest { get external() { return this.#external; } + + get isCancelled() { + if (this.#external === null) { + return true; + } + return op_http_get_request_cancelled(this.#external); + } } class CallbackContext { diff --git a/ext/http/http_next.rs b/ext/http/http_next.rs index 56c46de9258ba5..326478fe7c244e 100644 --- a/ext/http/http_next.rs +++ b/ext/http/http_next.rs @@ -700,6 +700,14 @@ fn set_response( http.complete(); } +#[op2(fast)] +pub fn op_http_get_request_cancelled(external: *const c_void) -> bool { + let http = + // SAFETY: op is called with external. + unsafe { clone_external!(external, "op_http_get_request_cancelled") }; + http.cancelled() +} + /// Returned promise resolves when body streaming finishes. /// Call [`op_http_close_after_finish`] when done with the external. #[op2(async)] diff --git a/ext/http/lib.rs b/ext/http/lib.rs index 6243804a1409f0..9d71e3ad3ccee8 100644 --- a/ext/http/lib.rs +++ b/ext/http/lib.rs @@ -113,6 +113,7 @@ deno_core::extension!( http_next::op_http_get_request_header, http_next::op_http_get_request_headers, http_next::op_http_get_request_method_and_url, + http_next::op_http_get_request_cancelled, http_next::op_http_read_request_body, http_next::op_http_serve_on, http_next::op_http_serve, diff --git a/tests/unit/serve_test.ts b/tests/unit/serve_test.ts index 439d71d5533fa0..9822a0ce2ab0b1 100644 --- a/tests/unit/serve_test.ts +++ b/tests/unit/serve_test.ts @@ -4270,3 +4270,32 @@ Deno.test({ assertEquals(hostname, "0.0.0.0"); await server.shutdown(); }); + +Deno.test({ + name: "AbortSignal aborted when request is cancelled", +}, async () => { + const { promise, resolve } = Promise.withResolvers(); + + let cancelled = false; + + const server = Deno.serve({ + hostname: "0.0.0.0", + port: servePort, + onListen: () => resolve(), + }, async (request) => { + request.signal.addEventListener("abort", () => cancelled = true); + assert(!request.signal.aborted); + await new Promise((resolve) => setTimeout(resolve, 3000)); // abort during waiting + assert(request.signal.aborted); + return new Response("Ok"); + }); + + await promise; + await fetch(`http://localhost:${servePort}/`, { + signal: AbortSignal.timeout(1000), + }).catch(() => {}); + + await server.shutdown(); + + assert(cancelled); +});