-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
StreamingTextResponse not compatible with Remix action function #199
Comments
Here is the polyfill code that is throwing the error |
I had to perform some unspeakable acts, but this package can be made compatible with remix actions now. import { createEventStreamTransformer, trimStartOfStreamHelper } from 'ai';
import { ReadableStream as PolyfillReadableStream } from 'web-streams-polyfill';
import { createReadableStreamWrapper } from '@mattiasbuelens/web-streams-adapter';
// @ts-expect-error bad types
const toPolyfillReadable = createReadableStreamWrapper(PolyfillReadableStream);
const toNativeReadable = createReadableStreamWrapper(ReadableStream);
export class StreamingTextResponse extends Response {
constructor(res: ReadableStream, init?: ResponseInit) {
const headers: HeadersInit = {
'Content-Type': 'text/plain; charset=utf-8',
...init?.headers,
};
super(res, { ...init, status: 200, headers });
this.getRequestHeaders();
}
getRequestHeaders() {
return addRawHeaders(this.headers);
}
}
function addRawHeaders(headers: Headers) {
// @ts-expect-error shame on me
headers.raw = function () {
const rawHeaders = {};
const headerEntries = headers.entries();
for (const [key, value] of headerEntries) {
const headerKey = key.toLowerCase();
if (rawHeaders.hasOwnProperty(headerKey)) {
// @ts-expect-error shame on me
rawHeaders[headerKey].push(value);
} else {
// @ts-expect-error shame on me
rawHeaders[headerKey] = [value];
}
}
return rawHeaders;
};
return headers;
}
function parseOpenAIStream(): (data: string) => string | void {
const trimStartOfStream = trimStartOfStreamHelper();
return (data) => {
// TODO: Needs a type
const json = JSON.parse(data);
// this can be used for either chat or completion models
const text = trimStartOfStream(json.choices[0]?.delta?.content ?? json.choices[0]?.text ?? '');
return text;
};
}
export function OpenAIStream(response: Response): ReadableStream<any> {
if (!response.ok || !response.body) {
throw new Error(`Failed to convert the response to stream. Received status code: ${response.status}.`);
}
const responseBodyStream = toPolyfillReadable(response.body);
// @ts-expect-error bad types
return toNativeReadable(responseBodyStream.pipeThrough(createEventStreamTransformer(parseOpenAIStream())));
} Just use this OpenAIStream instead of the one returned from There were two key issues:
Using these two patched functions in place of the |
Hi all, After some research / asking around, I believe this is a remix issue rather than an SDK one (but please let me know if you disagree!) Basically Remix uses
You shouldn't need to use the edge runtime though (even though we recommend it) so I'll look into the appropriate way to handle this. It seems like we'll either need to add polyfills to vercel/remix or submit something upstream. |
It would be really funny if this just worked 😅 I tried this while the action was on the same route as the component and it did not work. Let me try with the action on a dedicated resource route |
This does not work for local dev, which is a non starter for me unfortunately |
This is the same for HuggingFaceStream. Would love for this to work. @cephalization if you have any workarounds or end up using something else would love to know and would I can do the same for you 😄 |
My workaround reimplementation above works on local and hosted so I am happy with it for now. We will see if that changes when I try to upgrade the package in the future 😂 |
I've found these relevant upstream issues: |
@AvidDabbler copy paste the code from this comment into a new file, then import and use StreamingTextResponse from your new file. To get huggingfacestream working, you can likely tweak the openaistream I have shown with whatever hugging face specific logic there is, if any. Checkout the source code from this repo and see what you can make happen |
IDK how i missed that 🤦 |
@cephalization Hey how is your client side? I get a HTML stream as if you were doing a get request. |
My action looks just like it does at the beginning of this post. It is on its own route called I am using |
Tried for two days to work around it, and downgraded ai lib but I still get the full html as response. and that html response does not include the actual streaming values. I am using ChakraUI Im going to try this on a new project and see what happens. Thanks for sharing though. |
@yarapolana I can give you my code snippets. Here is my usage of the chat hook const { messages, append, reload, stop, isLoading, input, setInput } = useChat({
api: '/api/chat',
headers: {
'Content-Type': 'application/json',
},
initialMessages,
id,
body: {
id,
transcriptId: meeting.transcript.redisKey,
},
}); Here is my /api/chat route import type { ActionArgs } from '@vercel/remix';
import type { ChatCompletionRequestMessage } from 'openai-edge';
import { Configuration, OpenAIApi } from 'openai-edge';
import { z } from 'zod';
import { countMessageTokens, optimizeTranscriptModel } from 'llm';
import { transcriptQueries, redis } from '@/server/kv.server';
import { OpenAIStream, StreamingTextResponse } from '@/server/streamingTextResponse.server';
import { checkAuth } from '@/server/auth.utils.server';
const oConfig = new Configuration({
apiKey: process.env.OPENAI_API_KEY ?? '',
});
const openai = new OpenAIApi(oConfig);
export const action = async ({ request }: ActionArgs) => {
checkAuth(request);
const { messages, transcriptId } = z
.object({
messages: z.array(
z
.object({
content: z.string(),
role: z.union([z.literal('user'), z.literal('system'), z.literal('assistant')]),
})
.passthrough(),
),
transcriptId: z.string(),
})
.parse(await request.json());
// Load transcript set from Redis, turn it into a string
const transcriptArray = await transcriptQueries.getTranscriptArray(redis, { transcriptKey: transcriptId });
const transcript = transcriptArray;
const prompt = `HERE IS MY PROMPT`;
const { model, shortenedTranscript } = optimizeTranscriptModel(transcript, {
promptTokenBuffer: countMessageTokens(prompt),
});
const content = prompt.replace('{transcript}', shortenedTranscript.join('\n'));
const newMessages: ChatCompletionRequestMessage[] = [
{
role: 'system',
content,
},
...messages,
];
const response = await openai.createChatCompletion({
model,
messages: newMessages,
stream: true,
});
const stream = OpenAIStream(response);
const sResponse = new StreamingTextResponse(stream);
return sResponse;
}; |
@cephalization |
@yarapolana I ran into the same issue! very confusing. I think actions in a page route behave slightly differently than on their own route |
Hope this gets fixed soon! |
I haven't tested this yet but the ServerNodePolyfillOptions added in https://github.com/remix-run/remix/releases/tag/remix%401.19.0 look promising |
Nope, that option does not appear to do anything useful in local dev at least |
@cephalization I'm actually not quite sure how your earlier example works. I just upgraded to The server-side polyfill seems to override the native import {
ReadableStream as PolyfillReadableStream,
TransformStream as PolyfillTransformStream,
WritableStream as PolyfillfWritableStream,
} from "web-streams-polyfill/ponyfill";
console.log("api.ask-question");
console.log("read", ReadableStream === PolyfillReadableStream); // true
console.log("transform", TransformStream === PolyfillTransformStream); // false
console.log("write", WritableStream === PolyfillfWritableStream); // true One other weird thing. This works fine:
This hangs:
I've tried playing around with direct imports too, since all supported node versions have an official implementation ( |
@mindblight note for my example to work, you need to reimplement OpenAIStream and StreamingTextResponse like I've done at the top of this issue. This "fix" still works for the latest version of Hoping for some action on the remix side soon. |
@cephalization Ah, sorry - I was unclear. I'm not actually using the Rewriting it makes sense. Here's the line that's confusing to me:
From what I can tell, remix globally injects I'd like to find a slightly more generic solution that doesn't require rewriting specific stream implementations. I've tried both I thought |
Alright, I figured something out: // stream-compat.server.ts
import {
ReadableStream as NodeReadableStream,
TextDecoderStream as NodeTextDecoderStream,
TextEncoderStream as NodeTextEncoderStream,
TransformStream as NodeTransformStream,
WritableStream as NodeWritableStream,
} from "node:stream/web";
async function* iterStream<T>(body?: ReadableStream<T> | null) {
if (!body) {
return;
}
const reader = body.getReader();
while (true) {
const r = await reader.read();
if (r.done) {
break;
} else {
yield r.value;
}
}
}
// Ensures that custom streams are compatible with node streams
(global as any).ReadableStream = NodeReadableStream;
(global as any).TransformStream = NodeTransformStream;
(global as any).WritableStream = NodeWritableStream;
(global as any).TextDecoderStream = NodeTextDecoderStream;
(global as any).TextEncoderStream = NodeTextEncoderStream;
export const toNodeReadableStream = <T>(
polyStream: ReadableStream<T> | null | undefined
) => {
const iterator = iterStream(polyStream);
return new NodeReadableStream<T>({
async pull(controller) {
const r = await iterator.next();
if (r.done) {
controller.close();
} else {
controller.enqueue(r.value);
}
},
// This is *mostly* correct. There are slight type issues between the two streams
}) as ReadableStream<T>;
};
export const StreamCompat = {
toReadable: toNodeReadableStream,
// lib.dom.d.ts and web.d.ts typings are apparently incompatible.
// Remix routes use web.d.ts and .server.ts files use lib.dom.d.ts
// This exposes lib.dom.d.ts typings for the routes
TextEncoderStream,
TextDecoderStream,
}; In an action, you can now do StreamCompat.toReadable(response.body).pipeThrough(...) And everything should JustWork™️. In theory, you can now use the official I'm gonna cross-post on the issue in the remix-run. Hope this helps someone! |
@mindblight will give this a try later, thanks! Was really hoping to avoid using my patch at work. This looks more maintainable |
This is great stuff @mindblight. you and @cephalization seem to be "on the money". Any way to tag the remix team here? |
Any news? |
@Klingefjord have you tried something like #199 (comment)? |
@yarapolana I posted this in the remix issue that led me here. I haven't heard anything. The Remix 2 roadmap includes dropping polyfills by default, so I suspect that we won't get an official fix until that's released. Once the polyfills are gone, then this is no longer a problem |
Ok I figured out a better workaround. The issue is with OpenAI using node-fetch or Remix using When the OpenAI client returns a response, Vercel's AI passes a standard non-polyfilled The solutionRemove these polyfills in If the issue persists after the polyfills removal, OpenAI's package might also be causing it since they use node-fetch. In that case you can pass a custom const openai = new OpenAI({
apiKey: process.env.apiKey,
organization: process.env.organization,
fetch: globalThis.fetch
}); The temporary workaroundSince the problem is with import OpenAI from "openai";
import { OpenAIStream } from "ai";
export const action = async ({ request }: ActionArgs) => {
const { messages } = await request.json();
const openai = new OpenAI({
apiKey: process.env.apiKey,
organization: process.env.organization,
});
// This is the fix
const { TransformStream } = await import("web-streams-polyfill");
globalThis.TransformStream = TransformStream;
const response = await openai.chat.completions.create({
stream: true,
model: "gpt-3.5-turbo",
temperature: 1,
messages,
});
const stream = OpenAIStream(response);
return new StreamingTextResponse(stream);
} StreamingTextResponse: import { StreamingTextResponse as _StreamingTextResponse } from "ai";
// vercel/ai's StreamingTextResponse does not include request.headers.raw()
// which @vercel/remix uses when deployed on vercel.
// Therefore we use a custom one.
export class StreamingTextResponse extends _StreamingTextResponse {
constructor(res: ReadableStream, init?: ResponseInit) {
super(res, init);
this.getRequestHeaders();
}
getRequestHeaders() {
return addRawHeaders(this.headers);
}
}
const addRawHeaders = function addRawHeaders(headers: Headers) {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
headers.raw = function () {
const rawHeaders: { [k in string]: string[] } = {};
const headerEntries = headers.entries();
for (const [key, value] of headerEntries) {
const headerKey = key.toLowerCase();
// eslint-disable-next-line no-prototype-builtins
if (rawHeaders.hasOwnProperty(headerKey)) {
rawHeaders[headerKey].push(value);
} else {
rawHeaders[headerKey] = [value];
}
}
return rawHeaders;
};
return headers;
}; |
So Remix v2 is officially out now, wherein this is no longer an issue 🎉 |
I'm pretty sure the root problem lies within the Vercel adapter. Specifically, here: packages/vercel-remix/server.ts#L105. What do you think @TooTallNate, should one of us create a PR? EDIT: I was also getting the same error as @joelriveradev, but @MaxLeiter's solution worked for me! 🚀 |
Not sure if this helps, but the AI SDK uses the Node standard fetch. Have you tried setting the following in
More info: https://remix.run/docs/en/main/guides/single-fetch |
I have a remix app running on vercel, and was running into this issue trying to use the streamText method. What finally ended up working for me is the following: upgrade node to v20.x previously I was on v18.20.3
According to the remix 2.9.0 if you're on node 20.x you need should remove the installGlobals call in `vite.config.js, so I did. At this point, when running remix vite:dev locally streaming was working but I would run into this TypeError: First parameter has member 'readable' that is not a ReadableStream. error when deployed to vercel. What got it working on a vercel deploy was to provide the undici fetch to the openai provider, like this:
For anyone who is running into this issue, with streamText hope this helps |
Can confirm the explicit passing of |
When using the package in an action, like so,
The action returns an internal error
This appears to be due to some web streams workaround that remix installs. It even occurs when hosted on vercel. This error does not occur if I move all of this code into a dedicated express api but that defeats the purpose of me using remix as it would be my only external api call.
Anyone had luck fixing this? I've tried implementing the solutions here MattiasBuelens/web-streams-polyfill#93 with no luck.
I will try to produce a reproduction repo soon, but using any remix template should immediately reproduce the issue.
The text was updated successfully, but these errors were encountered: