kit icon indicating copy to clipboard operation
kit copied to clipboard

Allow using `fetch` with `agent` to reuse connections in server hooks

Open orenwang opened this issue 2 years ago • 6 comments

Describe the problem

In the docs for handleFetch hook it recommends to hit the API directly during SSR, which I agree:

export async function handleFetch({ request, fetch }) {
  if (request.url.startsWith('https://api.yourapp.com/')) {
    // clone the original request, but change the URL
    request = new Request(
      request.url.replace('https://api.yourapp.com/', 'http://123.123.123.123:9999/'),
      request
    );
  }
 
  return fetch(request);
}

However, if we could reuse TCP connections, this could be a lot faster than reestablishing every TCP connection like above. This is especially important if anyone, like me, wants to act like a proxy inside handle hook to avoid any CORS requests to an API. Currently we cannot reuse TCP connections with fetch because there is no such agent option we can pass, as specified in node-fetch docs, making every request way too slow.

Describe the proposed solution

Because handle and handleFetch are both server hooks, I assume the fetch we are using here is node-fetch, right? So we should be able to do something like this:

import http from 'node:http';

const httpAgent = new http.Agent({
	keepAlive: true
});

export async function handleFetch({ request, fetch }) {
  if (request.url.startsWith('https://api.yourapp.com/')) {
    // clone the original request, but change the URL
    request = new Request(
      request.url.replace('https://api.yourapp.com/', 'http://123.123.123.123:9999/'),
      request
    );
  }
 
  return fetch(request, {agent: httpAgent});
}

Alternatives considered

No response

Importance

i cannot use SvelteKit without it

Additional Information

I have tried to use fetch with keepalive option, but it does not have any effect. Plus, I don't feel this keepalive option is a "server-side" thing:

fetch(request, {keepalive: true});

orenwang avatar Oct 26 '22 06:10 orenwang

Currently we cannot reuse TCP connections with fetch because there is no such agent option we can pass, as specified in node-fetch docs, making every request way too slow.

Please note that we no longer use node-fetch for the most part, but primarily rely on undici. I would recommend consulting the undici docs. It would be helpful if you can let us know if it's an option they support or request the feature there first

benmccann avatar Nov 14 '22 23:11 benmccann

@benmccann Thank you for pointing this out! I looked into undici docs and found out there is actually a default 4s keep alive session for their implementation of fetch, and could be configured via a dispatcher. It seems SvelteKit not yet support configuring this.

orenwang avatar Nov 17 '22 07:11 orenwang

What do you mean by "does not support configuring this"? The fetch implementation used is that of undici, so you should be able to pass in whatever undici supports. If there are type errors this may be due to TypeScript type definitions not knowing about this field.

dummdidumm avatar Nov 17 '22 10:11 dummdidumm

@dummdidumm Do you mind show your code? Cuz I've tested it, regardless of typescript warnings of course, and it's not taking any effect.

Edit: by the way this is part of the code I've tested with. I'll look deeper when I have time, to find out what is actually causing dispatcher getting ignored.

import { Agent } from 'undici';

const agent = new Agent({
  // I've tried to increase the keep-alive time
  keepAliveTimeout: 100000 // 100s
  // And this, to test no session reuse
  // keepAliveTimeout: 1 // 1ms
});

export async function handleFetch({ event }) {
  if (request.url.startsWith('https://api.yourapp.com/')) {
    // clone the original request, but change the URL
    request = new Request(
      event.request.url.replace('https://api.yourapp.com/', 'http://123.123.123.123:9999/'),
      event.request
    );
  }
 
  return event.fetch(request, { dispatcher: agent });
}

orenwang avatar Nov 17 '22 10:11 orenwang

Note that we only use undici in Node-based environments (dev, preview, adapter-node, anything Lambda-based). Elsewhere, we use the platform's built-in fetch. Which platform are you observing this behaviour on, and how are you observing it?

Rich-Harris avatar Nov 17 '22 13:11 Rich-Harris

@Rich-Harris Thank you for providing this information!

Which platform are you observing this behaviour on

I just run npm run dev on my local machine, but calling a remote api.

and how are you observing it?

I put the fetch I imported from undici together with Sveltekit's fetch side by side, but using the same Agent as dispatcher. These two give me different elapsed time: only undici respects the keepAliveTimeout. Actual code can be found below.

If I refresh page in the browser every 5s, the server-side (where I run npm run dev) shows this: image

Code snippet for hooks.server.ts
import type { Handle } from '@sveltejs/kit';
import { BACKEND_BASE_API } from '$env/static/private';
import { Agent, fetch } from 'undici';

const agent = new Agent({
	// I've tried to increase the keep-alive time
	keepAliveTimeout: 100000 // 100s
	// And this, to test no session reuse
	// keepAliveTimeout: 1 // 1ms
});

export const handle: Handle = async ({ event, resolve }) => {
	if (event.url.pathname.startsWith('/api/')) {
		const req = new Request(
			event.request.url.replace(event.url.origin, BACKEND_BASE_API),
			event.request
		);
		for (const h of proxyHeadersIgnore) {
			req.headers.delete(h);
		}

		const t1 = new Date();
		const rKit = await event.fetch(req, { dispatcher: agent });
		const t2 = new Date();
		const rUndici = await fetch(req.url, { dispatcher: agent });
		const t3 = new Date();
		console.log(`kit: ${t2 - t1}ms`);
		console.log(`undici: ${t3 - t2}ms`);
		return rKit;
	}

	const response = await resolve(event);
	return response;
};

const proxyHeadersIgnore = [
	'accept',
	'cf-connecting-ip',
	'cf-ray',
	'content-md5',
	'host',
	'x-forwarded-host',
	'x-forwarded-port',
	'x-forwarded-proto',
	'connection'
];

orenwang avatar Nov 18 '22 01:11 orenwang

I'd also love to be able to put agents on fetches as well, but for a different use case. We have a proxy to the outside world, and without an agent, we can't make fetches out.

Our current work around is having a local node app, which we direct our current requests to, but that isn't exactly ideal, but given I know that it is undici now, I may be able to use the dispatch...

Either way, it would be good for something to be in the docs about this.

blairn avatar Mar 05 '23 22:03 blairn