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

Abort does not work with bun #3032

Closed
abaudhuin opened this issue Jun 24, 2024 · 4 comments · Fixed by #3042
Closed

Abort does not work with bun #3032

abaudhuin opened this issue Jun 24, 2024 · 4 comments · Fixed by #3042
Labels

Comments

@abaudhuin
Copy link

What version of Hono are you using?

4.2.4

What runtime/platform is your app running on?

bun

What steps can reproduce the bug?

With the following example inspired by the doc, I cannot get an onAbort to work using Bun, when switching to Node it works normally. I am testing with postman to make the request, and closing it in the ui.
Am I doing something wrong or something is broken ?

import { Hono } from "hono";
import { streamSSE } from "hono/streaming";
// import { serve } from "@hono/node-server";

const hono = new Hono();

let id = 0;
hono.get("t", (c) => {
  return streamSSE(c, async (stream) => {
    setTimeout(() => {
      stream.close();
    }, 10000);
    stream.onAbort(() => {
      console.log("aborted");
    });
    while (true) {
      try {
        await stream.writeSSE({
          data: `It is ${new Date().toISOString()}`,
          event: "time-update",
          id: String(id++),
        });
        await stream.sleep(1000);
      } catch (e) {
        console.error(e);
        break;
      }
    }
  });
});

// serve(hono);
export default hono;

What is the expected behavior?

'aborted' should be logged with bun and node.

What do you see instead?

'aborted' is not logged with bun

Additional information

No response

@usualoma
Copy link
Member

Hi @abaudhuin
Thank you for your report.

I'm considering a fix in #3042, but I'd like to investigate other runtimes and their behavior a bit more before merging.
You can use the #3042 branch if you need it now, which will work fine in the bun.

@abaudhuin
Copy link
Author

Thank you very much !
I've postponed my work needing this for now. I will wait for the fix to be merged.
Do you have an idea of the time it will take for this to be released?

@usualoma
Copy link
Member

@abaudhuin In #3042, I wrote a test and requested a review, so wait and see.

@abaudhuin
Copy link
Author

Thank you very much !!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants