remix-run / remix

Build Better Websites. Create modern, resilient user experiences with web fundamentals.
https://remix.run
MIT License
29.47k stars 2.48k forks source link

Piping `res.body` to TransformStream throws an error #6635

Open grabbou opened 1 year ago

grabbou commented 1 year ago

What version of Remix are you using?

Latest

Are all your remix dependencies & dev-dependencies using the same version?

Steps to Reproduce

  1. Fetch any URL
  2. PipeThrough res.body into TransformStream

Expected Behavior

Fetching and piping through the stream to TransformStream works without issues.

Actual Behavior

You will receive an error from Web polyfill used by Remix that "first parameter" (in this case, TransformStream) has a readable property that is not a ReadableStream.

First parameter has member 'readable' that is not a ReadableStream.

This is because TransformStream is not polyfilled along ReadableStream/WritableStream, as a result, native Node implementation is loaded.

When there are both polyfill and native streams circling around, this can lead to issues, e.g: https://github.com/MattiasBuelens/web-streams-polyfill/issues/93#issuecomment-925329286

grabbou commented 1 year ago

I found that when I polyfill TransformStream in injectGlobals or in server.ts, it still does not work:

import { installGlobals } from '@remix-run/node'
import { TransformStream } from '@remix-run/web-stream'

installGlobals()

global.TransformStream = TransformStream

But when I patch the source code directly, it works:

diff --git a/node_modules/ai/dist/index.js b/node_modules/ai/dist/index.js
index 46dcb84..b608338 100644
--- a/node_modules/ai/dist/index.js
+++ b/node_modules/ai/dist/index.js
@@ -70,6 +70,8 @@ __export(streams_exports, {
 });
 module.exports = __toCommonJS(streams_exports);

+const { Response } = require("@remix-run/node");
+const { TransformStream, ReadableStream } = require("@remix-run/web-stream");
 // streams/ai-stream.ts
 var import_eventsource_parser = require("eventsource-parser");
 function createEventStreamTransformer(customParser) {

Note I also have to import Response and ReadableStream explicitly from Remix packages, otherwise, it doesn't work while deployed in the Vercel environment.

wong2 commented 3 months ago

Any updates? Still facing this when using Remix with Vercel AI SDK 3.1

pokutuna commented 3 months ago

I have encountered the same problem. The description provides a good explanation, but I would like to add some information for users who are struggling like me.

As mentioned, this is an issue with the polyfill. For example, the following code does not work when using fetch from @remix-run/web-fetch:

const { fetch } = require("@remix-run/web-fetch");
(async () => {
  const res = await fetch("https://remix.run");
  res.body.pipeThrough(new TextDecoderStream()).getReader()
  // this throws:
  //   Uncaught TypeError: First parameter has member 'readable' that is not a ReadableStream.
})();

installGlobals from @remix-run/node installs this fetch globally. While this is necessary for compatibility, it also brings issues using pipeThrough(TransformStream) As a result, it can appear to cause errors in external libraries that use fetch.

If you are using Node 20 or later as a runtime, you can avoid this issue by using the native fetch. Removing installGlobals() or replacing it with installGlobals({ nativeFetch: true }).

There are several issues that I suspect might be caused by this problem:

According to these comments, the fetch polyfill will likely be discontinued in the next major version of Remix.

wong2 commented 3 months ago

@pokutuna I tried your solution, nativeFetch: true works in dev mode, but doesn't work after deploy with remix vite:build && remix-serve ./build/server/index.js. Do you have a solution?

amaany3 commented 3 months ago

@wong2 I encountered a similar issue. It seems that installGlobals does not have an effect when using remix-serve.

For preparation of using Node's built in fetch implementation, installing the fetch globals is now a responsibility of the app server. If you are using remix-serve, nothing is required. If you are using your own app server, you will need to install the globals yourself. https://remix.run/docs/en/main/start/v2#installglobals

Although it is unstable, you can avoid this issue by enabling single fetch. Since single fetch is being migrated to turbo-stream, it should resolve issues related to web-fetch and web-streams-polyfill.

wong2 commented 3 months ago

@amaany3 Thanks! I also found after enable single fetch it works!

pokutuna commented 3 months ago

Sorry for the confusion. I was talking about transforming streams.

Thank you for the follow-up. @amaany3