request.signal is never aborted

[adaboese]

I am trying to abort SSR when the original request is aborted, i.e.

export const loader = async ({ params, request }: LoaderFunctionArgs) => {
  return eventStream(request.signal, function setup(send) {
    const timer = setInterval(() => {
      console.log('TIME', request.signal.aborted);

      send({ data: new Date().toISOString(), event: 'time' });
    }, 1_000);

    return function clear() {
      console.log('CLEAR');

      clearInterval(timer);
    };
  });
};

But I just realized that CLEAR is never called and request.signal.aborted is always false – even after I navigate away from the page.

For what it is worth, I think this happens only is Vite dev server and it works as expected in production.

[lukasa1993]

vite dev has issues, if you use express template it works under express

[hi-ogawa]

Interesting. The difference between dev (default builtin vite dev server) and prod (remix-serve express server) might be caused by this PR:

• fix(remix-dev/vite): don't trigger request abort signal on dev server response close #8130

These two are mostly same except that the PR removed this part since it had an unintended effect on the usage of React renderToReadableStream on Node:

remix/packages/remix-express/server.ts

Lines 99 to 101 in 4968e3a

	// Abort action/loaders once we can no longer write a response
	let controller = new AbortController();
	res.on("close", () => controller.abort());

Now looking back the history of the code around AbortController, it looks like it wasn't that simple, for example, there is some platform-specific aspect of things discussed in:

• fix(remix-express,remix-vercel): fix request.signal #3626

(Actually I thought req.on("close", ...) is preferred over res.on("close", ...), but apparently the exact opposite happened in the PR.)

I think it's worth raising an issue. If you have a repro, then that would be great as well.

---

Probably my PR was premature (namely #8062 and #8130). I didn't check at that time, but there is some difference left in Response handling and the actual fix should probably take into account this part as well. Also it's totally possible that react-dom renderToReadableStream on NodeJs might be mis-used.

remix/packages/remix-express/server.ts

Line 117 in 4968e3a

export async function sendRemixResponse(

remix/packages/remix-dev/vite/node/adapter.ts

Lines 57 to 59 in 4968e3a

	// Adapted from solid-start's `handleNodeResponse`:
	// https://github.com/solidjs/solid-start/blob/7398163869b489cce503c167e284891cf51a6613/packages/start/node/fetch.js#L162-L185
	async function handleNodeResponse(webRes: Response, res: ServerResponse) {

[DanielBaulig]

Running into this exact same issue. Was there ever an issue filed? I'll open one if not.

[hi-ogawa]

I don't think it's been raised yet. It would be great if you can create one.
Also now that toNodeRequest is also used for cloudflare integration, so you can check how AbortController goes with react-dom/server's renderToReadableStream

remix/packages/remix-dev/vite/cloudflare-proxy-plugin.ts

Line 73 in 2e65318

let req = fromNodeRequest(nodeReq);

[DanielBaulig]

I crated a stackblitz repro and filed a bug: #9438

[brophdawg11]

This should be resolved via #9976