next.js icon indicating copy to clipboard operation
next.js copied to clipboard

Using ky with edge functions works on prod but fails on local Next.js dev server

Open transitive-bullshit opened this issue 2 years ago • 6 comments

Verify canary release

  • [X] I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
  Platform: darwin
  Arch: x64
  Version: Darwin Kernel Version 21.6.0: Mon Aug 22 20:17:10 PDT 2022; root:xnu-8020.140.49~2/RELEASE_X86_64
Binaries:
  Node: 16.18.0
  npm: 8.19.2
  Yarn: 1.22.19
  pnpm: 7.13.5
Relevant packages:
  next: 12.3.2-canary.29
  eslint-config-next: 12.3.1
  react: 18.2.0
  react-dom: 18.2.0

What browser are you using? (if relevant)

106.0.5249.119

How are you deploying your application? (if relevant)

next dev

Describe the Bug

ky is a small wrapper around fetch. The following edge function works fine in production: https://next-edge-test-yl6y.vercel.app/api/ky, but it produces an error when run locally with next dev:

import { NextRequest, NextResponse } from 'next/server'
import ky from 'ky'

export default async function handler(req: NextRequest) {
  const data = await ky
    .post('https://httpbin.org/post', {
      json: { test: true }
    })
    .json()
  console.log(data)

  return NextResponse.json(data)
}

export const config = {
  runtime: 'experimental-edge'
}

(source)

Visiting this edge function locally results in the error "TypeError: Request with GET/HEAD method cannot have body.":

Screen Shot 2022-10-18 at 1 43 09 PM

Expected Behavior

Visiting this edge function locally should work as it does in production, producing a valid JSON output: https://next-edge-test-yl6y.vercel.app/api/ky

This is presumably due to some inconsistency with how Next.js is polyfilling fetch or one of it's related globals. Is Next.js properly polyfilling all of the related globals as ky-universal does here? https://github.com/sindresorhus/ky-universal/blob/main/index.js

Link to reproduction

https://github.com/transitive-bullshit/next-edge-test

To Reproduce

  1. Clone https://github.com/transitive-bullshit/next-edge-test/blob/main/pages/api/ky.ts
  2. yarn install (or whatever package manager)
  3. yarn dev
  4. Visit http://localhost:3000/api/ky

transitive-bullshit avatar Oct 18 '22 17:10 transitive-bullshit

The Edge Runtime (that powers local development in Next.js) uses undici under the hood, which is closer to the fetch spec as node-fetch, which ky uses.

I suspect there is an issue because node-fetch notoriously uses Node.js' Readable, instead of the spec-compliant ResponseStream for body.

So there might be an issue around that. :thinking:

For some reason the following works.

- json: { test: true }
+ body: JSON.stringify({ test: true })

balazsorban44 avatar Oct 18 '22 21:10 balazsorban44

Thanks for the quick response, @balazsorban44.

To be clear though, ky does not use node-fetch. It uses globalThis.fetch. The link you mentioned is for ky-universal which is an optional adaptor for using ky within Node.js. Since this issue is focused on ky and not ky-universal, node-fetch shouldn't come into play. With that being said, it's possible that ky has mainly been tested on Node.js with node-fetch and that there is some incompatibility with undici.

For some reason the following works.

I can confirm that this change works as well for me locally, though I'm not sure why.

Maybe I should follow this up with some undici testing with ky that is independent of Next.js?

transitive-bullshit avatar Oct 18 '22 22:10 transitive-bullshit

Either way, it is rather odd that ky works as expected on Vercel's prod edge functions but fails to work with the local dev environment. This points to a possible bug with either undici (unlikely) or Vercel's edge-runtime (more likely), since it shows that the dev and prod environments aren't behaving the same.

transitive-bullshit avatar Oct 18 '22 22:10 transitive-bullshit

My bad, I actually noticed I was on the wrong repo. :facepalm:. I am investigating this still since there should be no difference between local/production deployment behavior anyway.

FWIW, Node.js now uses undici as it's fetch implementation since Node.js 18. (although still marked experimental).

balazsorban44 avatar Oct 18 '22 22:10 balazsorban44

I tried to understand what's happening.

If I create a minimal [email protected] and [email protected] interaction (that's pretty much what Edge runtime does) it works:

import { fetch, Headers, Request, Response } from 'undici'

globalThis.fetch = fetch
globalThis.Headers = Headers
globalThis.Request = Request
globalThis.Response = Response

const { default: ky } = await import('ky')

;(async () => {
  const data = await ky
    .post('https://httpbin.org/post', {
      json: { test: true }
    })
    .json()
  console.log(data)
})()

so I suspect the issue is actually in the Next.js transpilation layer, or in the Edge Runtime tweaks over undici. I have to go deep.

Kikobeats avatar Oct 20 '22 14:10 Kikobeats

Any updates on this @Kikobeats ?

baraeb92 avatar Nov 14 '22 06:11 baraeb92

I am running into a similar issue with an API proxy route. I am not using ky, just "native" (nextjs patched) fetch and runtime edge in the app router.

If I fix it to work locally then it fails when hosted on vercel edge function and vice versa.

I see this thread was from a year, did you guys figure out any solution?

ci-vamp avatar Sep 28 '23 11:09 ci-vamp

Probably related to: https://github.com/sindresorhus/ky/issues/531

Also, see: https://github.com/sindresorhus/ky/issues/516#issuecomment-1674303251

kdelmonte avatar Oct 17 '23 00:10 kdelmonte

I ran into this issue as well, and filed https://github.com/vercel/next.js/issues/57905 (I hadn't found this issue before filing mine).

I don't think this has anything to do with ky, other than an implementation detail of how they implement support for the json option - they happen to do it by cloning the original request and then adding the stringified JSON as the body: https://github.com/sindresorhus/ky/blob/main/source/core/Ky.ts#L201. However cloning the request is broken in edge runtime with dev server. you can see this with this minimal reproduction:

export default function Home() {
  const req1 = new Request('https://example.com', {
    method: 'POST',
    headers: { 'X-Example-Header': 'Foo' }
  });
  const req2 = new Request(req1);

  return <div>
    Request 1 method: {req1.method}
    <br/>
    Request 1 headers: {req1.headers}
    <br/>
    Request 2 method: {req2.method}
    <br/>
    Request 2 headers: {req2.headers}
  </div>
}

export const runtime = 'edge';

when run on dev server, it will render

Request 1 method: POST
Request 1 headers: x-example-headerFoo
Request 2 method: GET
Request 2 headers:

So the request method, headers (and everything else) is lost when using the copy constructor. This works correctly in all other environments (node on dev, edge or node in prod).

While the workaround is trivial as @balazsorban44 pointed out above (https://github.com/vercel/next.js/issues/41531#issuecomment-1283048552) it would still be nice to have this fixed - it's disconcerting to encounter these various differences in dev and production which seem to be fairly common for the edge runtime.

dkokotov avatar Nov 02 '23 08:11 dkokotov

@dkokotov

cloning the request is broken in edge runtime with dev server. you can see this with this minimal reproduction.

Not really. I ported the reproduction to a standalone Vercel Function:

import ky from 'ky'

export const config = { runtime: 'edge' }

export default async function handler () {
  const data = await ky
    .post('https://httpbin.org/post', {
      json: { test: true }
    })
    .json()

  console.log(data)

  return Response.json(data)
}

Deployed using vc --prod, and it worked:

❯ curl https://ky-edge.vercel.app/api
{"args":{},"data":"{\"test\":true}","files":{},"form":{},"headers":{"Accept":"application/json","Accept-Encoding":"gzip","Cdn-Loop":"cloudflare; subreqs=1","Cf-Connecting-Ip":"2a06:98c0:3600::103","Cf-Ew-Via":"15","Cf-Ray":"81fc16663085d34f-CDG","Cf-Visitor":"{\"scheme\":\"https\"}","Cf-Worker":"cloudflare-workers.vercel-infra-production.com","Content-Length":"13","Content-Type":"application/json","Host":"httpbin.org","User-Agent":"Vercel Edge Functions","X-Amzn-Trace-Id":"Root=1-65438a0b-2a8423b64ca3b3141d2eb62c","X-Middleware-Subrequest":"075acd0b65cd6c76742b7a7d913a7d1dd449e9b0","X-Vercel-Id":"cdg1::vxvsx-1698925067165-e55b48e08bb2"},"json":{"test":true},"origin":"2a06:98c0:3600::103, 172.71.126.140","url":"https://httpbin.org/post"}⏎

So the issue is still in the way ky and Next.js surface.

Kikobeats avatar Nov 02 '23 11:11 Kikobeats

@Kikobeats I did mention in my comment that the incorrect behavior only occurs on nextjs dev server (its in the part of my comment you quoted)

Your demonstration shows that it works in production, which I mentioned also.

This works correctly in all other environments (node on dev, edge or node in prod).

So yes it is definitely a Next.js issue, and can be reproduced with no involvement of ky. It is only a ky issue insofar as ky depends on the Request constructor working correctly when an existing request is passed in as the first argument - which does work correctly in all environments except edge runtime in nextjs dev server.

dkokotov avatar Nov 02 '23 17:11 dkokotov