vercel / remix

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

Error when using `VERCEL_REMIX_NATIVE_FETCH` with POST requests #124

Closed coji closed 3 months ago

coji commented 4 months ago

When enabling single fetch using VERCEL_REMIX_NATIVE_FETCH=1, POST requests result in an error. This occurs after deploying to Vercel.

Reproduction

Repository: https://github.com/coji/vercel-remix-singlefetch-test

Here's the relevant code causing the issue:

import { Form, useActionData } from '@remix-run/react';
import { unstable_defineAction as defineAction } from '@vercel/remix';

export const action = defineAction(async () => {
  return { message: 'Hello, world!' };
});

export default function Post() {
  const actionData = useActionData<typeof action>();

  return (
    <div className="px-4 py-2">
      <h1>Vercel Remix Single Fetch Test</h1>

      <Form method="POST">
        <button className="mt-2 rounded-md bg-indigo-600 px-2.5 py-1.5 font-semibold text-white shadow-sm hover:bg-indigo-500">
          Submit
        </button>

        {actionData && (
          <div className="mt-2 border rounded p-2">{actionData.message}</div>
        )}
      </Form>
    </div>
  );
}

Deployed URL: https://vercel-remix-singlefetch-test.vercel.app/

Steps to reproduce

  1. Deploy the code to Vercel with VERCEL_REMIX_NATIVE_FETCH=1 set in the environment variables.
  2. Visit the deployed URL and click the "Submit" button to trigger a POST request.

Expected behavior

The POST request should succeed, and the message "Hello, world!" should be displayed.

Actual behavior

The POST request fails, and the following client-side error is displayed:

Error: Unable to decode turbo-stream response from URL: https://vercel-remix-singlefetch-test.vercel.app/_root.data?index

Additionally, the following server-side error appears in the Vercel logs:

TypeError: RequestInit: duplex option is required when sending a body.
at new Request (/var/task/node_modules/.pnpm/undici@6.19.2/node_modules/undici/lib/web/fetch/request.js:539:15)
at createRemixRequest (file:///var/task/build/server/nodejs-eyJydW50aW1lIjoibm9kZWpzIn0/server-index.mjs:61:10)
at Server.default (file:///var/task/build/server/nodejs-eyJydW50aW1lIjoibm9kZWpzIn0/server-index.mjs:80:19)
at /opt/rust/nodejs.js:8:3973
at AsyncLocalStorage.run (node:async_hooks:346:14)
at /opt/rust/nodejs.js:8:3961
at AsyncLocalStorage.run (node:async_hooks:346:14)
at Server.<anonymous> (/opt/rust/nodejs.js:8:3949)
at Server.s (/opt/rust/nodejs.js:8:824)
at Server.emit (node:events:519:28)

Additional context

This issue seems to be related to the duplex option in the fetch API when using VERCEL_REMIX_NATIVE_FETCH=1. The error suggests that this option is required when sending a body, but it's not being set correctly in the current implementation.

It's worth noting that the Remix core has addressed this issue by setting duplex: 'half' when there's a request body. This can be seen in the Remix source code:

https://github.com/remix-run/remix/blob/main/packages/remix-server-runtime/single-fetch.ts#L151

export async function singleFetchAction(
  serverMode: ServerMode,
  staticHandler: StaticHandler,
  request: Request,
  handlerUrl: URL,
  loadContext: AppLoadContext,
  handleError: (err: unknown) => void
): Promise<{ result: SingleFetchResult; headers: Headers; status: number }> {
  try {
    let handlerRequest = new Request(handlerUrl, {
      method: request.method,
      body: request.body,
      headers: request.headers,
      signal: request.signal,
      ...(request.body ? { duplex: "half" } : undefined),
    });
    // ...

This suggests that the vercel/remix-builder implementation might need to incorporate a similar approach to resolve the current error.

Any assistance in resolving this issue would be greatly appreciated. Thank you!

TooTallNate commented 4 months ago

Hmmm, so far I am not able to reproduce. This deployment works as expected for example: https://vercel-remix-singlefetch-test-sandy.vercel.app

Is it possible that you are deploying with older versions of Remix dependencies which do not include the line of code that you referenced above?

coji commented 4 months ago

@TooTallNate

Thank you for looking into this. I appreciate your effort to reproduce the issue.

I can confirm that I'm using the latest version of Remix (2.10.2) in my project. This can be seen in the build logs, which show the following dependencies:

@remix-run/node 2.10.2
@remix-run/react 2.10.2
@remix-run/serve 2.10.2
@vercel/remix 2.10.2

Additionally, the build logs confirm that these versions are indeed being used during the deployment process. Regarding the Node.js runtime, I've set it to version 20.x in the Vercel project settings. The build logs also indicate this:

Warning: Detected "engines": { "node": ">=20.0.0" } in your package.json that will automatically upgrade when a new major Node.js Version is released.

Given this information, it seems that the deployment is using the latest versions of Remix and the correct Node.js runtime. Could there be any other factors that might be causing this issue?

I've uploaded the full build logs to a gist for your reference: https://gist.github.com/coji/f744cbcf4904d37d0a2b55ce30cdec44

If you need any additional information or if there's anything else I can provide to help diagnose this problem, please let me know.

coji commented 3 months ago

I'd like to provide an update on this issue:

  1. After removing the VERCEL_REMIX_NATIVE_FETCH=1 environment variable and redeploying the same code, the application now works without any issues.

  2. This resolution might be due to a recent PR merged into vercel/remix-builder yesterday: https://github.com/vercel/vercel/pull/11844

  3. It's unclear why @TooTallNate couldn't reproduce the issue a couple of days ago.

  4. Interestingly, if I redeploy with the VERCEL_REMIX_NATIVE_FETCH setting still in place, it still results in a 500 error due to the duplex requirement. The reason for this is not immediately apparent from the code.

  5. I'm pleased that single fetch is now working in the current state.

Given these developments, I'm wondering if it's correct to assume that we should no longer use VERCEL_REMIX_NATIVE_FETCH, and instead rely on Remix's future flags to control this behavior?

Any insights or clarification on these points would be greatly appreciated.

david-crespo commented 3 months ago

I am seeing the same issue! You can see it when typing into the search box here: https://docs-3s3upm0cx-oxidecomputer.vercel.app/

I will try turning VERCEL_REMIX_NATIVE_FETCH back off as you did. I turned Node 20 after I had turned that on, and it's possible all you need is Node 20 and no env var.

image

Edit: turning VERCEL_REMIX_NATIVE_FETCH back off does not work. Now everything is broken, not just POSTs, with the same old headers.getSetCookie is not a function error.

image
coji commented 3 months ago

I'd like to provide another update and ask for some clarification:

After removing the environment variable and deploying with Node.js 20, I attempted to use the Vercel AI SDK to make a streaming call to the OpenAI API. However, I encountered the following error:

APICallError [AI_APICallError]: Failed to process successful response
    at postToApi (file:///var/task/node_modules/.pnpm/@ai-sdk+provider-utils@1.0.2_zod@3.23.8/node_modules/@ai-sdk/provider-utils/dist/index.mjs:353:13)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    ... 3 lines matching cause stack trace ...
    at async file:///var/task/build/server/nodejs-eyJydW50aW1lIjoibm9kZWpzIn0/index.js:152:18
    at async Object.callRouteAction (/var/task/node_modules/.pnpm/@remix-run+server-runtime@2.10.2_typescript@5.5.3/node_modules/@remix-run/server-runtime/dist/data.js:37:16)
    at async /var/task/node_modules/.pnpm/@remix-run+server-runtime@2.10.2_typescript@5.5.3/node_modules/@remix-run/server-runtime/dist/single-fetch.js:88:20
    at async callLoaderOrAction (/var/task/node_modules/.pnpm/@remix-run+router@1.17.1/node_modules/@remix-run/router/dist/router.cjs.js:4713:16)
    at async /var/task/node_modules/.pnpm/@remix-run+server-runtime@2.10.2_typescript@5.5.3/node_modules/@remix-run/server-runtime/dist/single-fetch.js:83:20 {
  url: 'https://api.openai.com/v1/chat/completions',
  requestBodyValues: {
    model: 'gpt-4o',
    logit_bias: undefined,
    logprobs: undefined,
    top_logprobs: undefined,
    user: undefined,
    parallel_tool_calls: undefined,
    max_tokens: undefined,
    temperature: 0,
    top_p: undefined,
    frequency_penalty: undefined,
    presence_penalty: undefined,
    seed: undefined,
    messages: [ [Object] ],
    tool_choice: { type: 'function', function: [Object] },
    tools: [ [Object] ],
    stream: true,
    stream_options: { include_usage: true }
  },
  statusCode: 200,
  responseHeaders: {
    'alt-svc': 'h3=":443"; ma=86400',
    'cf-cache-status': 'DYNAMIC',
    'cf-ray': '8a581c6b3f66c58b-IAD',
    connection: 'close',
    'content-type': 'text/event-stream; charset=utf-8',
    date: 'Fri, 19 Jul 2024 04:54:00 GMT',
    'openai-organization': 'techtalk',
    'openai-processing-ms': '333',
    'openai-version': '2020-10-01',
    server: 'cloudflare',
    'set-cookie': '_cfuvid=Tq4kiuvPcbmkaGvrRLhasz.dUDMuUB0eCMUtlb2eEqk-1721364840756-0.0.1.1-604800000; path=/; domain=.api.openai.com; HttpOnly; Secure; SameSite=None',
    'strict-transport-security': 'max-age=15552000; includeSubDomains; preload',
    'transfer-encoding': 'chunked',
    'x-content-type-options': 'nosniff',
    'x-ratelimit-limit-requests': '5000',
    'x-ratelimit-limit-tokens': '800000',
    'x-ratelimit-remaining-requests': '4999',
    'x-ratelimit-remaining-tokens': '799975',
    'x-ratelimit-reset-requests': '12ms',
    'x-ratelimit-reset-tokens': '1ms',
    'x-request-id': 'req_b040620281d4d291c9cecf40cd3547d4'
  },
  responseBody: undefined,
  cause: TypeError: First parameter has member 'readable' that is not a ReadableStream.
      at assertReadableStream (/var/task/node_modules/.pnpm/web-streams-polyfill@3.3.3/node_modules/web-streams-polyfill/dist/ponyfill.js:466:19)
      at convertReadableWritablePair (/var/task/node_modules/.pnpm/web-streams-polyfill@3.3.3/node_modules/web-streams-polyfill/dist/ponyfill.js:4035:9)
      at ReadableStream.pipeThrough (/var/task/node_modules/.pnpm/web-streams-polyfill@3.3.3/node_modules/web-streams-polyfill/dist/ponyfill.js:4119:29)
      at file:///var/task/node_modules/.pnpm/@ai-sdk+provider-utils@1.0.2_zod@3.23.8/node_modules/@ai-sdk/provider-utils/dist/index.mjs:449:26
      at postToApi (file:///var/task/node_modules/.pnpm/@ai-sdk+provider-utils@1.0.2_zod@3.23.8/node_modules/@ai-sdk/provider-utils/dist/index.mjs:342:20)
      at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
      at async OpenAIChatLanguageModel.doStream (file:///var/task/node_modules/.pnpm/@ai-sdk+openai@0.0.36_zod@3.23.8/node_modules/@ai-sdk/openai/dist/index.mjs:275:50)
      at async _retryWithExponentialBackoff (file:///var/task/node_modules/.pnpm/ai@3.2.27_react@18.3.1_svelte@4.2.18_vue@3.4.32_typescript@5.5.3__zod@3.23.8/node_modules/ai/dist/index.mjs:41:12)
      at async streamObject (file:///var/task/node_modules/.pnpm/ai

It appears that undici is not being used in this scenario.

I noticed in the Remix documentation for single fetch, there's a section about the deprecated fetch polyfill:

If you are using Node 20+, remove any calls to installGlobals() and use Node's built-in fetch (this is the same thing as undici).

Given this information, I have a few questions:

  1. Does the vercel/remix-builder need to be updated to not call installGlobals at all when both Node.js 20 and single fetch are being used?

  2. Is there a way to ensure that undici (or Node's native fetch for Node.js 20+) is being used correctly in this setup?

@TooTallNate, your insights on these points would be greatly appreciated. Is there anything else I should be considering or any additional information I can provide to help diagnose this issue?

jahvi commented 3 months ago

Also experiencing this issue using the provided repo from OP even after upgrading to v2.10.3.

All server side POST requests throw the RequestInit: duplex option is required when sending a body. error, making the same request client side works as expected.

dwene commented 3 months ago

I am also having this issue, although it's odd and sometimes difficult to reproduce, because it happens intermittently.

dwene commented 3 months ago

For anyone else who is struggling with this, I was able to get native fetch to work by running the function in this gist at the top of entry.server.tsx. Hope this helps.

https://gist.github.com/michalmo/d62e9a2e001183ec1df1bcac74980abb

david-crespo commented 3 months ago

So from that gist, it sounds like Vercel should try adding the following here?

if (
  init.body &&
  init.body instanceof IncomingMessage &&
  init.duplex === undefined
) {
  initPatched = {
    ...init,
    duplex: "half",
  };
}
TooTallNate commented 3 months ago

This has been fixed, and will be available in the next Vercel CLI release.

If you would like to try out the fix now, you can temporarily set the env var VERCEL_CLI_VERSION with a value of https://vercel-lhv8s3ozl.vercel.sh/tarballs/vercel.tgz. Remember to remove this env var once the new Vercel CLI is released.