SSE: Invalid state: Controller is already closed

[JasonColeyNZ]

Hi, loving being able to use middleware with RR7, but I have an issue with SSE and Hono, not sure what is happening.

I have been using Remix and remix-live-loader (https://github.com/moishinetzer/remix-live-loader) for sometime, and have this week making the changes for RR7, using your hono-server implementation for the middleware ability.

I am getting intermittent errors using remix-live-loader and react-router-hono-server together, it seems as though connections are not being closed when navigating back and forth.

I have a repo here to take a look at, its the example given by remix-live-loader with your hono-server added.
https://github.com/JasonColeyNZ/remix-live-loader

If you open two windows and navigate back and forth between the task lists on the left and mark and change task completion, eventually (after 10-20) changes and switching back and forth between the two tabs you will get the controller error.

Any chance you can take a look and maybe help find out what is happening?

[rphlmr]

Hello 👋

I will look at it but I'm pretty sure that you need the same implementation as https://github.com/rphlmr/drizzle-lab/blob/b07f58e8a40aaffcadb62e243810a3a179aafbad/apps/cli/visualizer/routes/watch.ts#L17C1-L31C63

export async function loader({ request }: LoaderFunctionArgs) {
  const abortController = new AbortController();
  abortController.signal.addEventListener("abort", () =>
    abortController.abort(),
  );

  // simplified code

  return eventStream(
    AbortSignal.any([request.signal, abortController.signal]),
    (send: SendFunction) => {
      function notify(event: EventType) {
        try {
          send({ event, data: new Date().toISOString() });
        } catch (cause) {
          if (
            cause instanceof Error &&
            !cause.message.includes("Controller is already closed")
          ) {
            console.error("Failed to send SSE event", cause);
          }
        }
      }

      // simplified code
    },
  );
}

I'm not sure about the reason. I added that for the same error, the request signal isn't triggered for some reason.

[JasonColeyNZ]

Are you sure why, because remix hasn't needed this in the past, and RR7 doesn't need it either, works very well in fact with the implementation that remix-live-loader uses. It's only when hono is added that the issues appear. if you remove hono server it all works as expected again?

[rphlmr]

Yep. I have this issue with Hono since Vite.
I don't know why, maybe a difference between ExpressJS and Hono.
Since it fixes my issues, I haven't dig deeper.
I should try without Remix utils, just in case.

[JasonColeyNZ]

If all we are doing is adding a try/catch around the send, then surely we will have a memory leak?
I have a console that shows the emitter listenerCount on the loaders and this just seems to be going up and up.
emitter.listenerCount()
HMR seems to be the main cause here, each time a save occurs in a stream modification, the old handles are not being freed.

[JasonColeyNZ]

Interesting I can't actually get the emitter count over 3, or two windows, my only concern is with the potential with many users hitting pages at the same time?

[rphlmr]

Thanks for investigating!

I am not sure about a fix but I've added it to my todo

[JasonColeyNZ]

Yeah with the sample I made, with three windows I now have an emitter count of 6 on one of the slugs, so its just getting higher and higher. I did find somewhere that nextjs were having the same error and their 'solution' was to patch react itself.

vercel/next.js@f2009f3
https://x.com/Kingsley_codes/status/1808578766375113027

I did read also that nextjs 14 had similar issues.

also undici were looking into this, I tried Node 22 but this didn't help
nodejs/undici#1564

[rphlmr]

So, I tried your example and I was able to reproduce it.

We can better handle the error and maintain a proper number of connections with:

import { eventStream } from "remix-utils/sse/server";
import { emitter } from "./emitter.server";

export function createEventStream(request: Request, eventName: string) {
  const abortController = new AbortController();
  abortController.signal.addEventListener("abort", () =>
    abortController.abort()
  );

  return eventStream(
    AbortSignal.any([request.signal, abortController.signal]),
    (send) => {
      const handle = () => {
        try {
          console.log("listenerCount: ", emitter.listenerCount(eventName));

          send({
            data: String(Date.now()),
          });
        } catch (error) {
          if (
            error instanceof Error &&
            !error.message.includes("Controller is already closed")
          ) {
            console.error("Failed to send SSE event", error);
          } else {
            // Remove dead listeners
            emitter.removeListener(eventName, handle);
          }
        }
      };

      emitter.addListener(eventName, handle);

      return () => {
        emitter.removeListener(eventName, handle);
      };
    }
  );
}

You will see a "peak" in listenerCount, but it is just the first time after an HMR update.