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

[jvaill]

Closes: #9976 (comment)

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: nodejs/node#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>

[changeset-bot]

🦋 Changeset detected

Latest commit: aaab009

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 16 packages

Name	Type
@remix-run/server-runtime	Patch
@remix-run/cloudflare	Patch
@remix-run/deno	Patch
@remix-run/dev	Patch
@remix-run/node	Patch
@remix-run/react	Patch
@remix-run/testing	Patch
@remix-run/cloudflare-pages	Patch
@remix-run/cloudflare-workers	Patch
@remix-run/architect	Patch
@remix-run/express	Patch
@remix-run/serve	Patch
create-remix	Patch
remix	Patch
@remix-run/css-bundle	Patch
@remix-run/eslint-config	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[remix-cla-bot]

Hi @jvaill,

Welcome, and thank you for contributing to Remix!

Before we consider your pull request, we ask that you sign our Contributor License Agreement (CLA). We require this only once.

You may review the CLA and sign it by adding your name to contributors.yml.

Once the CLA is signed, the CLA Signed label will be added to the pull request.

If you have already signed the CLA and received this response in error, or if you have any questions, please contact us at [email protected].

Thanks!

- The Remix team

[remix-cla-bot]

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

[jvaill]

Fyi - the underlying problem appears to be a bug in undici:

nodejs/node#55428