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

AbortSignals from Deno fetch do not work on Windows #26924

Open
bdashore3 opened this issue Nov 19, 2024 · 9 comments
Open

AbortSignals from Deno fetch do not work on Windows #26924

bdashore3 opened this issue Nov 19, 2024 · 9 comments
Assignees
Labels
bug Something isn't working correctly windows Related to Windows platform

Comments

@bdashore3
Copy link

bdashore3 commented Nov 19, 2024

Version: Deno 2.0.6

Hi there,
I'm trying to create a Hono server with a disconnect handler. When I cancel the request from my Postman client, the abort signal from the raw Request object never changes to true. This code works on both Node and Bun, so I think this is an issue with Deno's fetch API. See example below (requires Hono)

import { Hono } from 'hono'

const app = new Hono()

async function stall(stallTime = 3000) {
  await new Promise(resolve => setTimeout(resolve, stallTime));
}

app.get('/hello', async (c) => {

  // Should show up when hitting Cancel in Postman
  c.req.raw.signal.addEventListener("abort", () => {
    console.log("Aborted");
  })

  // Simulate a long-running task
  await stall(10000);
  return c.text("Hello from Hono!");
})

Deno.serve(app.fetch)
@bartlomieju
Copy link
Member

@bdashore3 are you sure you are on 2.0.6? It works fine for me and this was fixed in #26781

@bartlomieju bartlomieju added the needs info needs further information to be properly triaged label Nov 19, 2024
@bdashore3
Copy link
Author

Hi @bartlomieju!

Yes, I'm on Deno v2.0.6. Here's my console info. Should've mentioned that I'm on Windows. Also, that example does not work on my system.

deno 2.0.6 (stable, release, x86_64-pc-windows-msvc)
v8 12.9.202.13-rusty
typescript 5.6.2

@bdashore3
Copy link
Author

bdashore3 commented Nov 19, 2024

However, this does work on the macOS version of Deno. I cannot test Linux, but it should be working there as well if it works on Mac. So, this is a Windows specific issue that maybe fell through testing?

deno 2.0.6 (stable, release, aarch64-apple-darwin)
v8 12.9.202.13-rusty
typescript 5.6.2

@bdashore3 bdashore3 changed the title AbortSignals from Deno fetch do not work AbortSignals from Deno fetch do not work on Windows Nov 19, 2024
@Seally
Copy link

Seally commented Nov 19, 2024

When I ran your example, the "Aborted" message was correctly printed to the console. I accessed the page on Firefox and closed the tab before the timeout. I'm on Windows 10.

Does the non-aborted response work as expected for you?

It's a bit of a stab in the dark, but is there some security software on your Windows system that whitelists Node and Bun but not Deno (or something along those lines)? I remember someone from ages back had issues because their security software had sandboxed Deno but not Node.

@bdashore3
Copy link
Author

bdashore3 commented Nov 19, 2024

@Seally I am also on Windows 11, not 10. I tried your suggestion and also reinstalled deno via winget instead. Now, the aborted print shows up.

However, there still seems to be some differences in behavior in this example between Windows and macOS. On Windows, the abort handler only triggers when the long-running task (await stall) is completed, which results in the Aborted print happening after a long delay. However, on Mac, aborting the request immediately fires the event (which is what should happen).

I am not sure if my code is incorrect here. If it isn't, then there's something going on with how the event loop is prioritizing request cancellations on Windows vs other operating systems or there's something different happening with Windows request APIs.

To create a testable environment, I'd recommend using Postman or curl instead of a browser window.

@bartlomieju bartlomieju added bug Something isn't working correctly windows Related to Windows platform and removed needs info needs further information to be properly triaged labels Nov 19, 2024
@bdashore3
Copy link
Author

bdashore3 commented Nov 19, 2024

Here's a more fine-grained snippet. I removed the stall function and used Deno's delay function. This is a common pattern for handling disconnects using FastAPI in python.

import { Hono, HonoRequest } from 'hono'
import { logger } from "hono/logger"
import { delay } from "@std/async"

const app = new Hono()
app.use(logger())

async function checkDisconnect(req: HonoRequest) {
  while(!req.raw.signal.aborted) {
    await delay(1000);
  }

  console.log("Aborted");
  return;
}

app.get('/hello', async (c) => {

  // Simulate a long-running task
  // The cancel should immediately happen and finish the request
  // On Mac, this behavior is correct, but on Windows, the cancellation never fires
  await Promise.race([
    delay(10000),
    checkDisconnect(c.req),
  ]);

  return c.text("Hello from Hono!");
})

Deno.serve(app.fetch)

Basically, when cancelling the request from the client, the race will end and the function will finish. This behavior is correct on Mac, but not on Windows where the long-running task runs to its entirety.

@bartlomieju
Copy link
Member

Thanks @bdashore3, though I think this example is a bit flawed; having that while loop in checkDisconnect will always "block" for at least 1s. I think the original repro is more useful.

That said, we'll look into why it's not working on Windows as expected. Are you using WSL or anything else worth mentioning?

@bdashore3
Copy link
Author

bdashore3 commented Nov 20, 2024

Fair enough. The main point I was trying to make there is that the request doesn't get cancelled until the delay is completed (which shouldn't happen). Thanks for looking into it! Hopefully the differing behaviors between OSes can be fixed.

And no, I'm using purely Windows 11. WSL is not on my system at the moment.

@littledivy
Copy link
Member

littledivy commented Nov 25, 2024

Why not use the signal event handler? This triggers even if the request is cancelled before delay:

import { delay } from "jsr:@std/async";

function checkDisconnect(req) {
  const { promise, resolve } = Promise.withResolvers<void>();
  req.signal.addEventListener("abort", () => {
    resolve();
    console.log("Aborted");
  });

  return promise;
}

Deno.serve(async (req) => {
  await Promise.race([
    delay(10000),
    checkDisconnect(req)
  ]);

  return new Response("Ok");
});

On Mac, this behavior is correct, but on Windows, the cancellation never fires

This also happens on Mac because your while loop doesn't do another check for atleast 1 sec. I tried on M1 macOS 15.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly windows Related to Windows platform
Projects
None yet
Development

No branches or pull requests

4 participants