refactor: rejig hyper-connect usage of fetch

[TillaTheHun0]

There seems to be a new issue when using hyper-connect on Vercel, more specifically passing a Request object to undici, which is what hyper-connect does.

See vercel/vercel#9690 for background

While that issue is being addressed, I am going to have to update hyper-connect to -- not do that. I think I can take the Request object and pull off it's parts and then pass it to fetch

[TillaTheHun0]

This was a doozy to debug. I've filed an issue with Vercel vercel/vercel#9690

Firstly, this error only occurs on deployments to Vercel, it does not happen locally. So debug iterations involved committing and pushing to GitHub, waiting on Vercel to deploy, then looking at the function logs in Vercel.

I was able to replicate this on all combinations of Node 16 and 18 and NextJS 12 and 13.

I set up an endpoint that called directly into an instance of hyper-connect and was able to replicate the error. So I started diving into the details.

From the stacktrace, it seemed that a Request was somehow being passed into a URL, and URL does not know how to parse the Request object, and so throws.

hyper-connect is fundamentally a client built around Web Standards: Request and fetch. Basically, each hyper-connect api just builds out a Request object using the input parameters, then passes that Request object into fetch. This is absolutely valid use of fetch and Request, according to the the fetch spec:

fetch(new Request('https://foo.bar', {...})) // this is valid, according to the spec

fetch ('https://foo.bar', { ... }) // this is also valid, according to the spec

Both of these usages of fetch are implemented by libraries such as node-fetch and undici that aim to provide spec compliant fetch. hyper-connect uses undici's fetch implementation on node.

So I started looking into where URL is used within undici fetch, and it turns out that URL is used on that 2nd usage of fetch shown above -- when a string or URL is passed as the first parameter. So the question now was "why isn't undici code, that properly handles passing a Request to fetch being fired?". I knew NextJS polyfilled fetch, but wasn't sure how, and I assumed that NextJS' fetch polyfill might be using Vercel's fetch, which I already knew did not support passing Request to fetch. But hyper-connect directly imports fetch from undici via import { fetch, Request } from 'undici', so how could hyper-connect be using the NextJS polyfill, so I started looking into NextJS/Vercel code around the fetch polyfill and found something really weird.

There are tons of references to undici in NextJS source code, in particular references to a precompiled version of undici that is part of the NextJS source code. Then I found this webpack code that seems to be hooking into Node's module resolution. I don't know for sure, i'm not so familiar with these Node require hook apis, but it looks like this is telling Node , when require('undici') is encountered, to instead load the custom NextJS precompiled version of undici. Okay, so NextJS is seemingly overwriting Node resolving undici to resolve their own version of undici. So that would explain how importing unidici, which I know implements our fetch usage, could somehow still break. So now the question was "does NextJS precompiled undici allow passing a Request object?". And looking at the code, whether or not to load the custom undici seems to be driven off of a feature flag. So I did a test.

I created two identical Lambdas and deployed them onto Vercel:

One passing a string:

import type { NextApiRequest, NextApiResponse } from 'next';
import { fetch, Request } from 'undici'


export default async function (_req: NextApiRequest, res: NextApiResponse): Promise<void> {
    return fetch('https://jsonplaceholder.typicode.com/todos/1')
        .then(res => res.json())
        .then(res.json)
        .catch((err) => {
            console.error(err);
            res.json(err);
        });
}

And one passing Request to fetch from undici (NextJS' undici)

import type { NextApiRequest, NextApiResponse } from 'next';
import { fetch, Request } from 'undici'


export default async function (_req: NextApiRequest, res: NextApiResponse): Promise<void> {
    return fetch(new Request('https://jsonplaceholder.typicode.com/todos/1'))
        .then(res => res.json())
        .then(res.json)
        .catch((err) => {
            console.error(err);
            res.json(err);
        });
}

Notice that these lambdas don't reference hyper-connect AT ALL, only undici. When I hit each of these lambdas:

The first Lambda succeeds.

The second Lambda fails, with the error we were getting from our hyper-connect calls.

This proves that NextJS is overwriting undici fetch with it's own version of fetch that is not spec compliant, in the way that hyper-connect expects, causing the error we have been seeing with the hyper-connect calls.

Solution

I have filed an issue on Vercel. They are seemingly overwriting a module resolution as part of their cloud environment. And because it's as the Node module resolution level, there doesn't seem to be a way to get around it without changing our own code.

I suspect that will take a while for the Vercel team to address, so I have altered the behavior of hyper-connect to instead use fetch and Request in a way that Vercel's undici does support.

[kaloyanBozhkov]

I have been struggling with this issue for 1 month now, any idea how we can push forward a solution? Initially I solved the problem by forcing Next.JS to use node-fetch instead of undici, but Next.JS had an update which dropped node-fetch entirely in favour of undici and the workaround I was using is not helpful anymore.

I created related issues on undici, vercel and next.js in hopes to drive momentum but this issue seems to be extremely slow to resolve.

I tried replicating locally, sadly I was not able to. It's such a tricky edge case that seems to only error on deployments on
vercel 😭

[TillaTheHun0]

@kaloyanBozhkov I see your comment on the Vercel issue I filed. It's a different error than what we were seeing, but could very well be caused by the same underlying thing: Vercel overwriting undici with their own"version", and their "version" not supporting how you're using it.

I had to dig into undici and Vercel source code to verify that Vercel actually was doing this. Unfortunately, because this is a module resolution hook put in place by Vercel, and they aren't expressing any interest in solving this anytime soon, we had to change our code to use undici differently, in a way that Vercel's "version" was okay with.

I can't speak to your specific issue, other than suggest that you follow the same approach: dig into your code and your dependency code ie. @trpc/client and see if you can figure out a way to change your usage of undici until you can get it working on Vercel. It may require asking maintainers of @trpc/client for help. Then continue to comment on vercel/vercel#9690 that this is an issue for you. If enough folks comment on the issue, maybe Vercel will address it.