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

Breaking: use Request and Response objects in endpoints and hooks #3384

Merged
merged 41 commits into from
Jan 19, 2022

Conversation

Rich-Harris
Copy link
Member

@Rich-Harris Rich-Harris commented Jan 17, 2022

This PR introduces a number of breaking changes that

  1. lay the foundation for streaming request/response bodies
  2. enable multipart form handling (including file uploads)
  3. better align SvelteKit with modern platforms that deal with Request and Response objects natively

How to update your app

Hooks (handle, handleError and getSession) and endpoints previously received a proprietary Request object:

interface Request<Locals = Record<string, any>, Body = unknown> {
  url: URL;
  method: string;
  headers: RequestHeaders;
  rawBody: RawBody;
  params: Record<string, string>;
  body: ParameterizedBody<Body>;
  locals: Locals;
}

Instead, they now receive a RequestEvent:

export interface RequestEvent<Locals = Record<string, any>> {
  request: Request; // https://developer.mozilla.org/en-US/docs/Web/API/Request
  url: URL; // https://developer.mozilla.org/en-US/docs/Web/API/URL
  params: Record<string, string>;
  locals: Locals;
}

method and headers are no longer necessary as they exist on the request object. (url is still provided, since the URL object contains conveniences like url.searchParams.get('foo'), whereas request.url is a string.)

Updating hooks

The resolve function passed to handle now returns a Promise<Response>; handle must do likewise.

A rewritten handle hook might look like this:

-export async function handle({ request, resolve }) {
-  const response = await resolve(request);
+export async function handle({ event, resolve }) {
+  const response = await resolve(event);

-  if (response.headers['content-type'].startsWith('text/html')) {
-    response.body = response.body.replace(/cloud/g, 'butt');
+  if (response.headers.get('content-type').startsWith('text/html')) {
+    const body = await response.text();
+    return new Response(body.replace(/cloud/g, 'butt'), response);
  }
  
  return response;
}

(This example illustrates the new APIs but also demonstrates a way in which rewriting HTML is now less efficient; we may need to add a transformPage option or something to satisfy that use case in a more streamlined way.)

Updating endpoints

Similarly, handlers receive a RequestEvent. Most GET handlers will be unchanged, but any handler that needs to read the request body will need to update:

-export async function post({ params, body }) {
+export async function post({ params, request }) {
+ const body = await request.formData(); // or request.json(), etc
  await do_something_with(params, body);
  return { status: 201 };
}

In future we will add helpers for handling request bodies in streaming fashion cross-platform

Handlers do not need to return a Response object, but they can. Specifically, the headers property can be any form that can be passed to the Headers constructor...

headers = new Headers(); // undefined is ok
headers = new Headers({ 'content-type': 'text/html' }); // so is a POJO
headers = new Headers(headers); // so is a `Headers` instance (or a `Map` etc, for that matter)

...which means that since a Response object has a compliant status, headers and body, you can do this sort of thing:

// src/routes/api/[...path].js
export const get = ({ params }) => fetch(`https://myapi.com/${params.path}`);

Updating svelte.config.js

If you're using the headers, host or protocol config options, you should remove them. Most adapters will automatically set the correct URL without the help of these options; in other cases like adapter-node the options have been moved into the adapter options.

How to update your custom adapter

The interface for adapters has changed as well — the app.render function now takes a Request and returns a Promise<Response>.

On platforms that use these objects natively (like Cloudflare Workers and Deno) this means you can delete some code. On Lambda-like and Node-like platforms you must create the Request and handle the Response yourself — consult the adapter-node and adapter-netlify examples:

/** @type {import('polka').Middleware} */
const ssr = async (req, res) => {
let request;
try {
request = await getRequest(base || get_base(req.headers), req);
} catch (err) {
res.statusCode = err.status || 400;
return res.end(err.reason || 'Invalid request body');
}
setResponse(res, await app.render(request));
};

const rendered = await app.render(
new Request(rawUrl, {
method: httpMethod,
headers: new Headers(headers),
body: rawBody
})
);

(Note that your adapter may need to expose options to configure the base URL (or protocol/host header) like adapter-node does, following the removal of the headers/host/protocol config options.)

What's next

After these API changes are in place, we can implement support for streaming request/response bodies, which will enable handling of large files and so on (at least on platforms that support streaming, i.e. not Lambda).

Original PR comment

Breaking change — changes the app.render signature, which adapters will need to accommodate

  • app.render should accept a Request as input
  • app.render should return a Response
  • allow endpoints to return a Headers object (enables one-line fetch proxying)
  • need to expose helpers for converting between Request and Response and node req/res
  • handle streaming bodies (this bit will be finicky since we realistically need to support node readable streams, at least until the standard ReadableStream object is supported everywhere — node-fetch uses node streams under the hood instead of web streams)
  • docs

This doesn't yet tackle file uploads (#70), that can happen in a follow-up I think. It also doesn't mean that Svelte components somehow support streaming, this is purely about simplifying the API and enabling streamed endpoint request/response bodies

Follow-ups:

  • Handle large file uploads (#issuecomment-1015569661)
  • Figure out if we polyfill ReadableStream to enable streamed responses in a cross-platform way

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpx changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

@changeset-bot
Copy link

changeset-bot bot commented Jan 17, 2022

🦋 Changeset detected

Latest commit: 7a5a03a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages
Name Type
@sveltejs/kit Patch
@sveltejs/adapter-cloudflare Patch
@sveltejs/adapter-cloudflare-workers Patch
@sveltejs/adapter-netlify Patch
@sveltejs/adapter-node Patch
@sveltejs/adapter-vercel Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@netlify
Copy link

netlify bot commented Jan 17, 2022

✔️ Deploy Preview for kit-demo canceled.

🔨 Explore the source changes: 7a5a03a

🔍 Inspect the deploy log: https://app.netlify.com/sites/kit-demo/deploys/61e86ce8ece61400079c3eed

@Rich-Harris
Copy link
Member Author

A realisation: if the signature of app.render changes to (request: Request) => Promise<Response>, then the adapter needs to construct a full URL (since request.url is always absolute). In the case of modern environments that implement Request natively, this is already taken care of; you just pass the request unchanged.

Netlify provides a rawUrl property on the event (I think this is a non-standard extension to the Lambda signature, though I could be wrong). Vercel simulates a Node request, but the protocol is always https and the host is provided in the host header so we can reliably construct a full URL. Vercel edge functions, when we're able to support them, expose Request objects like Cloudflare Workers (and Deno).

So of the officially supported adapters, the only one that can actually make use of the host and protocol config (or headers.host/headers.protocol) is adapter-node. I suspect adapter-node is the odd one out, since most adapters are tailored for a specific environment where the full URL is already available or easy to construct reliably.

The conclusion is that we should remove headers/host/protocol from the config, and put it inside adapter-node instead.

@Rich-Harris
Copy link
Member Author

Another realisation: since request bodies can only be consumed once, and since there are valid reasons to need to control how it's consumed (#831, #70 (comment)), we can't realistically parse the body before calling handlers.

And the endpoint-level config idea (#70 (comment)) is a non-starter unfortunately, since we would need to parse the body before identifying which endpoint(s) matched the request.

So I've changed my mind a bit about the API — I think we need to expose the underlying Request object to handlers:

-export async function post({ url, method, headers, rawBody, body, params, locals }) {
+export async function post({ request, url, params, locals }) {
+  const body = await request.json();
  // ...
}

(Since request contains method and headers, we can ditch those as well as rawBody and body.)

And this would in fact enable file uploads, albeit in a suboptimal way:

export async function post({ request }) {
  const data = await request.formData();
  await send_to_s3(data.get('video'));

  // ...
}

For handling large files this isn't ideal, since it buffers all the data in memory. Essentially what we'd eventually need is a way to handle uploads in streaming fashion — Remix has parseMultiPartFormData which is close to what I'm imagining, except that it appears to be Node-only. Something like this:

import { multipart } from '$app/somewhere';

export async function post({ request }) {
  for await (const { name, value } of multipart(request)) {
    if (name === 'video') {
      await send_to_s3(value.name, value.type, value.stream());
    }
  }

  // ...
}

Happily, that doesn't need to be part of this PR.

BliiTzZ added a commit to BliiTzZ/realworld that referenced this pull request Mar 2, 2022
A PR in SvelteKit introduces a number of breaking changes that

lay the foundation for streaming request/response bodies
enable multipart form handling (including file uploads)
better align SvelteKit with modern platforms that deal with `Request` and `Response` objects natively

Hooks (`handle`, `handleError` and `getSession`) and endpoints previously received a proprietary `Request` object:
```ts
interface Request<Locals = Record<string, any>, Body = unknown> {
  url: URL;
  method: string;
  headers: RequestHeaders;
  rawBody: RawBody;
  params: Record<string, string>;
  body: ParameterizedBody<Body>;
  locals: Locals;
}
```
Instead, they now receive a `RequestEvent`:

```ts
export interface RequestEvent<Locals = Record<string, any>> {
  request: Request; // https://developer.mozilla.org/en-US/docs/Web/API/Request
  url: URL; // https://developer.mozilla.org/en-US/docs/Web/API/URL
  params: Record<string, string>;
  locals: Locals;
}
```
`method` and `headers` are no longer necessary as they exist on the `request` object. (`url` is still provided, since the `URL` object contains conveniences like `url.searchParams.get('foo')`, whereas `request.url` is a string.)

To access the request body use the text/json/arrayBuffer/formData methods, e.g. `body = await request.json()`.
See sveltejs/kit#3384 for details
@WeAreELIC
Copy link

So, is it possible to get the rawBody data now? I'm using Stripe's API and need the raw request body from an endpoint's post function. Help!

@j2l
Copy link

j2l commented Mar 24, 2022

I was lead here by a svelte error when trying to update packages of https://github.com/timsonner/sveltekit-system-call
Noob question (because I tried hard to update it according to this post, but it's obscure to me): how to adapt the module?
Thank you svelte gurus!

@benwoodward
Copy link

I'm also struggling to figure out how to get the rawBody value, I need it for a mux.com webhook

@kylebuildsstuff
Copy link

I'm also struggling to figure out how to get the rawBody value, I need it for a mux.com webhook

I'm not sure about mux.com but I was able to get the rawBody for Stripe's checkout webhook with this:

function toBuffer(ab: any) {
  const buf = Buffer.alloc(ab.byteLength);
  const view = new Uint8Array(ab);
  for (let i = 0; i < buf.length; ++i) {
    buf[i] = view[i];
  }
  return buf;
}

export async function post(event: RequestEvent<Record<string, string>>) {
   ...
  const preRawBody = await event.request.arrayBuffer();
  const rawBody = toBuffer(preRawBody);
  
  try {
    stripeEvent = stripe.webhooks.constructEvent(
      rawBody,
      stripeSignature,
      process.env.STRIPE_WEBHOOK_SECRET
    );
  } ...
 }

@benwoodward
Copy link

Thanks @KTruong008, this is how I fixed my code (you may find using .buffer() simpler)

export async function post({ request }) {
  const rawBody = await request.buffer();
  const body = JSON.parse(rawBody.toString());
  const signature = request.headers.get('mux-signature') || '';
  const eventData = body.data;

  let isValidSignature = false;

  try {
    isValidSignature = Webhooks.verifyHeader(
      rawBody,
      signature,
      MUX_WEBHOOK_SECRET
    );

@WeAreELIC
Copy link

@benwoodward — You can also get the raw body from request.text(). Ex: const rawBodyText = await request.text();

@comverser
Copy link

comverser commented Apr 8, 2022

I need help finding right replacement.

Original took.ts was:

import type { Handle } from '@sveltejs/kit';

export const handle: Handle = async ({ request, resolve }) => {
	if (request.url.searchParams.has('_method')) {
		request.method = request.url.searchParams.get('_method').toUpperCase();
	}

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

=> request in handle has been replaced with event. See https://github.com/sveltejs/kit/pull/3384 for details

So I changed

import type { Handle } from '@sveltejs/kit';

export const handle: Handle = async ({ event, resolve }) => {
	if (event.url.searchParams.has('_method')) {
		event.request.method = event.url.searchParams.get('_method').toUpperCase();
	}

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

But it is not working giving this error

event.method has been replaced by event.request.method. See https://github.com/sveltejs/kit/pull/3384 for details
Error: event.method has been replaced by event.request.method. See https://github.com/sveltejs/kit/pull/3384 for details
    at Object.get (file:https:///workspace/voicebot/.svelte-kit/runtime/server/index.js:2532:10)
    at Module.api (/workspace/voicebot/src/routes/services/todo/todos/_api.ts:10:24)
    at get (/workspace/voicebot/src/routes/services/todo/todos/index.json.ts:5:12)
    at render_endpoint (file:https:///workspace/voicebot/.svelte-kit/runtime/server/index.js:150:25)
    at resolve (file:https:///workspace/voicebot/.svelte-kit/runtime/server/index.js:2656:17)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async Object.handle (/workspace/voicebot/src/hooks.ts:8:24)
    at async respond (file:https:///workspace/voicebot/.svelte-kit/runtime/server/index.js:2564:20)
    at async fetch (file:https:///workspace/voicebot/.svelte-kit/runtime/server/index.js:1697:17)
    at async load (index.svelte:5:20)
Unexpected token < in JSON at position 0
SyntaxError: Unexpected token < in JSON at position 0
    at JSON.parse (<anonymous>)
    at Proxy.<anonymous> (file:https:///workspace/voicebot/.svelte-kit/runtime/server/index.js:1797:21)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async load (index.svelte:12:28)
    at async load_node (file:https:///workspace/voicebot/.svelte-kit/runtime/server/index.js:1826:12)
    at async respond$1 (file:https:///workspace/voicebot/.svelte-kit/runtime/server/index.js:2191:15)
    at async render_page (file:https:///workspace/voicebot/.svelte-kit/runtime/server/index.js:2392:19)
    at async resolve (file:https:///workspace/voicebot/.svelte-kit/runtime/server/index.js:2657:11)
    at async Object.handle (/workspace/voicebot/src/hooks.ts:8:24)
    at async respond (file:https:///workspace/voicebot/.svelte-kit/runtime/server/index.js:2564:20)

Would you please tell me how to fix the problem?

@benwoodward
Copy link

@hyunduk0206 I recommend asking this kind of question on StackOverflow or the svelte discord. However, I would just console.log out the values to see what's contained within the parameters.

@comverser
Copy link

@benwoodward Thank you for your reply. I will not ask this kind of question here, except for this question.

I have tried to print the values, but I couldn't due to error. But I can write down the part of code that is related to the value.

import type { Request } from '@sveltejs/kit';
import PrismaClient from '$lib/prisma';

const prisma = new PrismaClient();

export const api = async (request: Request, data?: Record<string, unknown>) => {
	let status = 500;
	let body = {};

	switch (request.method.toUpperCase()) {
		case 'GET':
			status = 200;
			body = await prisma.todo.findMany();
			break;
		case 'POST':
			status = 201;
			body = await prisma.todo.create({
				data: {
					created_at: data.created_at as Date,
					done: data.done as boolean,
					text: data.text as string
				}
			});
			break;
		case 'DELETE':
			status = 200;
			body = await prisma.todo.delete({
				where: {
					uid: request.params.uid
				}
			});
			break;
		case 'PATCH':
			status = 200;
			body = await prisma.todo.update({
				where: {
					uid: request.params.uid
				},
				data: {
					done: data.done,
					text: data.text
				}
			});
			break;

		default:
			break;
	}

	if (request.method.toUpperCase() !== 'GET' && request.headers.accept !== 'application/json') {
		return {
			status: 303,
			headers: {
				location: '/'
			}
		};
	}

	return {
		status,
		body
	};
};

@comverser
Copy link

comverser commented Apr 11, 2022

I solved my problem by following the offical doc (https://kit.svelte.dev/docs/routing#endpoints-http-method-overrides).

@nmfrankel
Copy link

Adding the info of how to read request body data to kit.svelte.com would be quite useful, it took me over a day to find this thread.

@csjh
Copy link

csjh commented Sep 7, 2022

Adding the info of how to read request body data to kit.svelte.com would be quite useful, it took me over a day to find this thread.

https://kit.svelte.dev/docs/web-standards#fetch-apis-request ?

Could be in a clearer place alongside +server.js and +page.server.js docs though

@nmfrankel
Copy link

Yes, my app only has +server.js routes for the db access and I couldn't find it anywhere when including +server.js in the search. There's documentation and an example for url.params, but I think request.body is at least as important to require an example.

I found the answer from this thread, Svelte gave the link as a console error, but all the attempts with request.formData() (as suggested on StackOverflow), only gave the error without the link.

@xpluscal
Copy link

I’m currently getting “Invalid Request Body” when deployed and uploading a file using a Named Action through a form. Any ideas why?

@xpluscal
Copy link

kit.svelte.com

Hey does this solution still work for you? I'm having issues with the arraybuffer. It seems that you can get the body via response.text() but the stripe verification still fails.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet