remix icon indicating copy to clipboard operation
remix copied to clipboard

fix(server-runtime: fix request signal not aborting after request is garbage collected

Open jvaill opened this issue 5 months ago • 8 comments

~Closes: https://github.com/remix-run/remix/pull/9976#issuecomment-2373035840~ Edit: There appears to be a different bug with signals not working at all in production.

Update (Sep 26)

In development, it appears that globals aren't being automatically installed and Request with node ends up coming from undici. undici currently has a bug where parent signals aren't properly followed: https://github.com/nodejs/node/issues/55428

In production, globals are installed and Request comes from web-fetch. This version of Request does not have the above issue.

It seems like globals should be installed in development as well? Manually invoking installGlobals() changes the Request object and solves this issue.

Original:

Testing Strategy:

Add remix-utils and the following routes. Start the dev server and open /time in a bunch of different tabs. Refresh the tabs randomly, close them, create new ones, etc, and eventually you'll trigger the GC in the right way where SSE connection closed ${number of open conns} will be logged to the console with the number of open connections being greater than the number of open tabs. A good way to see if you've triggered this is just to close all of them and see if the logged connection count is greater than 0.

routes/sse.time.ts

import { LoaderFunctionArgs } from "@remix-run/node"
import { eventStream } from "remix-utils/sse/server"

let conns = 0;

export async function loader({ request }: LoaderFunctionArgs) {
  conns++;
  return eventStream(request.signal, function setup(send) {
    const timer = setInterval(() => {
      send({ event: "time", data: new Date().toLocaleTimeString() })
    }, 1000)

    return function clear() {
      conns--;
      console.log("SSE connection closed", conns);
      clearInterval(timer)
    }
  })

routes/time.tsx

import { useEventSource } from "remix-utils/sse/react"

export default Route(){
  let time = useEventSource("/sse/time", { event: "time" })
  if (!time) {
    time = new Date().toLocaleTimeString()
  }

return <p>{time}</p>

jvaill avatar Sep 26 '24 04:09 jvaill