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

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

Open
1 task done
transitive-bullshit opened this issue Oct 18, 2022 · 11 comments · May be fixed by vercel/edge-runtime#688
Open
1 task done
Labels
bug Issue was opened via the bug report template. Runtime Related to Node.js or Edge Runtime with Next.js.

Comments

@transitive-bullshit
Copy link

transitive-bullshit commented Oct 18, 2022

Verify canary release

  • 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 transitive-bullshit added the bug Issue was opened via the bug report template. label Oct 18, 2022
@balazsorban44 balazsorban44 added the Runtime Related to Node.js or Edge Runtime with Next.js. label Oct 18, 2022
@balazsorban44
Copy link
Member

balazsorban44 commented Oct 18, 2022

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. 🤔

For some reason the following works.

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

@transitive-bullshit
Copy link
Author

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

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.

@balazsorban44
Copy link
Member

balazsorban44 commented Oct 18, 2022

My bad, I actually noticed I was on the wrong repo. 🤦. 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).

@Kikobeats
Copy link
Member

Kikobeats commented Oct 20, 2022

I tried to understand what's happening.

If I create a minimal ky@0.31.3 and undici@5.11.0 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.

@baraeb92
Copy link

Any updates on this @Kikobeats ?

@ci-vamp
Copy link

ci-vamp commented Sep 28, 2023

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?

@kdelmonte
Copy link

Probably related to: sindresorhus/ky#531

Also, see: sindresorhus/ky#516 (comment)

@dkokotov
Copy link

dkokotov commented Nov 2, 2023

I ran into this issue as well, and filed #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 (#41531 (comment)) 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.

@Kikobeats
Copy link
Member

Kikobeats commented Nov 2, 2023

@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.

@dkokotov
Copy link

dkokotov commented Nov 2, 2023

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue was opened via the bug report template. Runtime Related to Node.js or Edge Runtime with Next.js.
Projects
None yet
7 participants