Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Piping res.body to TransformStream throws an error #6635

Open
1 task done
grabbou opened this issue Jun 18, 2023 · 7 comments
Open
1 task done

Piping res.body to TransformStream throws an error #6635

grabbou opened this issue Jun 18, 2023 · 7 comments
Labels
bug:unverified feat:fetch Issues related to @remix-run/web-fetch runtime:node

Comments

@grabbou
Copy link

grabbou commented Jun 18, 2023

What version of Remix are you using?

Latest

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

  • Yes

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: MattiasBuelens/web-streams-polyfill#93 (comment)

@grabbou
Copy link
Author

grabbou commented Jun 18, 2023

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.

@grabbou grabbou changed the title Inject TransformStream as global Piping res.body to TransformStream throws an error Jun 18, 2023
@brophdawg11 brophdawg11 added runtime:node feat:fetch Issues related to @remix-run/web-fetch labels Jun 20, 2023
@brophdawg11 brophdawg11 added this to v2 Aug 3, 2023
@brophdawg11 brophdawg11 moved this to Backlog in v2 Aug 3, 2023
@brophdawg11 brophdawg11 removed this from v2 Sep 5, 2023
@wong2
Copy link

wong2 commented Jun 2, 2024

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

@pokutuna
Copy link

pokutuna commented Jun 3, 2024

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
Copy link

wong2 commented Jun 3, 2024

@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
Copy link

amaany3 commented Jun 3, 2024

@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
Copy link

wong2 commented Jun 4, 2024

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

@pokutuna
Copy link

pokutuna commented Jun 4, 2024

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

Thank you for the follow-up. @amaany3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug:unverified feat:fetch Issues related to @remix-run/web-fetch runtime:node
Projects
None yet
Development

No branches or pull requests

6 participants