Skip to content

Commit

Permalink
feat(remix-server-runtime): strip body from request before callin…
Browse files Browse the repository at this point in the history
…g `loader`s (#3207)

feat: strip body from request before calling loaders

Since loaders map to "GET" and "HEAD" requests, those by definition can't have a body and therefore there is no reason to pass it in to use-code. This also avoids exclusive locks on the body when multiple loaders are executed in parallel.

Changes:
- Action recieves OG request and can choose to read the body.
- handleDocumentRequest recieves OG request, if the action read the body it will be locked and `bodyUsed` will be true. Atempting to read the body again will throw
- Loaders recieve a copy of the OG request without the body.
- Updated docs
  • Loading branch information
jacob-ebey authored May 17, 2022
1 parent 97ed0e1 commit ba6de76
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 21 deletions.
8 changes: 4 additions & 4 deletions docs/decisions/0002-do-not-clone-request.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ Status: accepted

## Context

To allow multiple loaders / actions to read the body of a request, we have been cloning the request before forwarding it to user-code. This is not the best thing to do as some runtimes will begin buffering the body to allow for multiple consumers. It is also goes against "the platform" that states a request body should only be consumed once.
To allow multiple loaders / actions to read the body of a request, we have been cloning the request before forwarding it to user-code. This is not the best thing to do as some runtimes will begin buffering the body to allow for multiple consumers. It also goes against "the platform" that states a request body should only be consumed once.

## Decision

Do not clone requests before they are passed to user-code (loaders, actions, handleDocumentRequest, handleDataRequest, etc.).
Do not clone requests before they are passed to user-code (actions, handleDocumentRequest, handleDataRequest), and remove body from request passed to loaders. Loaders should be thought of as a "GET" / "HEAD" request handler. These request methods are not allowed to have a body, therefore you should not be reading it in your Remix loader function.

## Consequences

If you are reading the request body in both an action and a loader this will now fail. Loaders should be thought of as a "GET" / "HEAD" request handler. These request methods are not allowed to have a body, therefore you should not be reading it in your Remix loader function.
Loaders always receive a null body for the request.

If you wish to continue reading the request body in multiple places for a single request against recommendations, consider using `.clone()` before reading it; just know this comes with tradeoffs.
If you are reading the request body in both an action and handleDocumentRequest or handleDataRequest this will now fail as the body will have already been read. If you wish to continue reading the request body in multiple places for a single request against recommendations, consider using `.clone()` before reading it; just know this comes with tradeoffs.
16 changes: 0 additions & 16 deletions packages/remix-server-runtime/responses.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,19 +69,3 @@ export function isRedirectResponse(response: Response): boolean {
export function isCatchResponse(response: Response) {
return response.headers.get("X-Remix-Catch") != null;
}

export function extractData(response: Response): Promise<unknown> {
let contentType = response.headers.get("Content-Type");

if (contentType && /\bapplication\/json\b/.test(contentType)) {
return response.json();
}

// What other data types do we need to handle here? What other kinds of
// responses are people going to be returning from their loaders?
// - application/x-www-form-urlencoded ?
// - multipart/form-data ?
// - binary (audio/video) ?

return response.text();
}
7 changes: 6 additions & 1 deletion packages/remix-server-runtime/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -293,13 +293,18 @@ async function handleDocumentRequest({
);
}

let loaderRequest = new Request(request.url, {
...request,
body: null,
});

let routeLoaderResults = await Promise.allSettled(
matchesToLoad.map((match) =>
match.route.module.loader
? callRouteLoader({
loadContext,
match,
request,
request: loaderRequest,
})
: Promise.resolve(undefined)
)
Expand Down

0 comments on commit ba6de76

Please sign in to comment.