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

[Feat]: Better DX for Abort events #1753

Open
2 tasks done
tri2820 opened this issue Feb 3, 2025 · 3 comments
Open
2 tasks done

[Feat]: Better DX for Abort events #1753

tri2820 opened this issue Feb 3, 2025 · 3 comments
Labels
contribution welcome we would appreciate a PR enhancement New feature or request

Comments

@tri2820
Copy link

tri2820 commented Feb 3, 2025

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 😯

When the request is aborted, the abort event is not fired within server API route.

Example server API route

import type { APIEvent } from "@solidjs/start/server";
export async function GET(event: APIEvent) {

    event.request.signal.addEventListener("abort", () => {
        console.log('abort detected')
    })

    let i = 0;
    while (i < 20) {
        console.log('looping', i++, event.request.signal.aborted)
        await new Promise(resolve => setTimeout(resolve, 1000))
    }

    return new Response(null, { status: 200 })
}

Example browser triggers

<button
onClick={() => {
  controller = new AbortController();
  fetch("/api/test", {
    method: "GET",
    signal: new AbortController().signal,
  })
    .then((response) => {
      console.log("response", response);
    })
    .catch((error) => {
      console.log("error", error);
    });
}}
>
Call
</button>
<button
onClick={() => {
  console.log("controller", controller);
  controller?.abort("I am aborting this request");
  controller = undefined;
}}
>
Abort
</button>

Expected behavior 🤔

When user cancel the request in browser, the abort event should be fired.

Steps to reproduce 🕹

Steps:

  1. Create an API route
  2. Add abort listener inside the API route
  3. In the browser, fetch & abort
  4. Notice that the abort event is not fired inside the API route

Context 🔦

I'm building an AI chat app that streams the token from the AI provider back to the user.
Currently when the user clicks aborting the request, the api route could not detect that and keep streaming from the AI provider, wasting token.

Not sure if related but Next.js used to have a similar issue vercel/next.js#48682

Your environment 🌎

Linux denpa 6.12.10-arch1-1 #1 SMP PREEMPT_DYNAMIC Sat, 18 Jan 2025 02:26:57 +0000 x86_64 GNU/Linux

bun --version
1.2.0

node --version
v23.4.0

"dependencies": {
    "@solidjs/meta": "^0.29.4",
    "@solidjs/router": "^0.15.0",
    "@solidjs/start": "^1.0.11",
    "solid-js": "^1.9.2",
    "vinxi": "^0.4.3"
  },
@tri2820 tri2820 added the bug Something isn't working label Feb 3, 2025
@tri2820 tri2820 changed the title [Bug?]: Abort event does not fire on API route [Bug?]: Abort event is not fired on API route Feb 3, 2025
@tri2820
Copy link
Author

tri2820 commented Feb 6, 2025

Update: After some digging I found out that we can listen to the close event on nativeEvent for this.

import { getRequestEvent } from "solid-js/web";

export async function GET() {
    const event = getRequestEvent()!;
    event.nativeEvent.node.req.addListener('close', () => {
        console.log('Client closed connection');
    })

    await new Promise(resolve => setTimeout(resolve, 5000));

    return new Response(`I'm alive!`, {
        status: 200,
    });
}

Nevertheless it would be good if event.request.signal.addEventListener("abort", ...) is supported

@atilafassina
Copy link
Member

Thanks for this issue and the thorough explanation, @tri2820
Imho, that's definitely something that's worth having a better DX around (ping @katywings for her 2cents).


If you have time, it would be great if you could add a reproduction to our /tests workspace.

You can start the PR with the failing test and then we can have a clear path moving forward.

Meanwhile, this workaround is worthy of the docs, so I'm pinging @LadyBluenotes in case she wants to create an issue there too :)

As soon as I can get my hands on trying the workaround, I can help writing the guide (but you're also ofc welcomed to do so if you want)

@atilafassina atilafassina added enhancement New feature or request and removed bug Something isn't working labels Feb 12, 2025
@atilafassina atilafassina changed the title [Bug?]: Abort event is not fired on API route [Feat]: Better DX for Abort events Feb 12, 2025
@atilafassina atilafassina added the contribution welcome we would appreciate a PR label Feb 12, 2025
@katywings
Copy link
Contributor

@tri2820 Thank you for bringing this up. I also think that having a better DX around abort sounds like a sensible addition 💯.

The first question coming to my mind is if event.nativeEvent.node.req.addListener('close', ...) is the proper way to do this from a nitro/h3 perspective. Maybe @pi0 can give us some insight about this 🙂. Touching event.node directly is usually not recommended by h3, so I wonder if there is some "more official" way to do this, or if it already is planned for a future update?

A similar question was asked in the Nitro Discussions under nitrojs/nitro#1541 but is unanswered 🙈.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution welcome we would appreciate a PR enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants