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

fix(data-loader): unify the response logic of the Loader in SSR #6720

Merged
merged 3 commits into from
Jan 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/tricky-chairs-press.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@modern-js/plugin-data-loader': patch
'@modern-js/runtime': patch
---

feat: unify the response logic of the Loader in SSR
feat: 统一 SSR 中 Loader 的响应逻辑
72 changes: 54 additions & 18 deletions packages/cli/plugin-data-loader/src/cli/createRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,28 +38,45 @@ const handleRedirectResponse = (res: Response) => {
return res;
};

const handleDeferredResponse = async (res: Response) => {
if (
res.headers.get('Content-Type')?.match(CONTENT_TYPE_DEFERRED) &&
res.body
) {
return await parseDeferredReadableStream(res.body);
}
return res;
const isDeferredResponse = (res: Response) => {
return (
res.headers.get('Content-Type')?.match(CONTENT_TYPE_DEFERRED) && res.body
);
};

const isRedirectResponse = (res: Response) => {
return res.headers.get('X-Modernjs-Redirect') != null;
};

const isErrorResponse = (res: Response) => {
return res.headers.get('X-Modernjs-Error') != null;
};

function isOtherErrorResponse(res: Response) {
return (
res.status >= 400 &&
res.headers.get('X-Modernjs-Error') == null &&
res.headers.get('X-Modernjs-Catch') == null &&
res.headers.get('X-Modernjs-Response') == null
);
}

const isCatchResponse = (res: Response) => {
return res.headers.get('X-Modernjs-Catch') != null;
};

const handleErrorResponse = async (res: Response) => {
if (isErrorResponse(res)) {
const data = await res.json();
const error = new Error(data.message);
error.stack = data.stack;
throw error;
}
return res;
const data = await res.json();
const error = new Error(data.message);
error.stack = data.stack;
throw error;
};

const handleNetworkErrorResponse = async (res: Response) => {
const text = await res.text();
const error = new Error(text);
error.stack = undefined;
throw error;
};

export const createRequest = (routeId: string, method = 'get') => {
Expand All @@ -75,11 +92,30 @@ export const createRequest = (routeId: string, method = 'get') => {
res = await fetch(url, {
method,
signal: request.signal,
}).catch(error => {
throw error;
});

res = handleRedirectResponse(res);
res = await handleErrorResponse(res);
res = await handleDeferredResponse(res);
if (isRedirectResponse(res)) {
return handleRedirectResponse(res);
}

if (isErrorResponse(res)) {
return await handleErrorResponse(res);
}

if (isCatchResponse(res)) {
throw res;
}

if (isDeferredResponse(res)) {
return await parseDeferredReadableStream(res.body!);
}

// some response error not from modern.js
if (isOtherErrorResponse(res)) {
return await handleNetworkErrorResponse(res);
}

return res;
};
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/plugin-data-loader/src/runtime/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export function sanitizeError<T = unknown>(error: T) {
process.env.NODE_ENV !== 'development' &&
process.env.NODE_ENV !== 'test'
) {
const sanitized = new Error('Unexpected Server Error');
const sanitized = new Error(error.message || 'Unexpected Server Error');
sanitized.stack = undefined;
return sanitized;
}
Expand Down
3 changes: 2 additions & 1 deletion packages/cli/plugin-data-loader/src/runtime/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,8 @@ export const handleRequest = async ({
});
}
const cost = end();

// add response header for client know if the response is from modern server
response.headers.set('X-Modernjs-Response', 'yes');
onTiming?.(`${LOADER_REPORTER_NAME}-navigation`, cost);
} catch (error) {
if (isResponse(error)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ export const createRequestHandler: CreateRequestHandler =

const initialData = await runBeforeRender(context);

// Support data loader to return status code
// Support data loader to return `new Response` and set status code
if (
context.routerContext?.statusCode &&
context.routerContext?.statusCode !== 200
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,18 +125,17 @@ export const routerPlugin = (
return interrupt(routerContext);
}

// requestHandler.ts also has same logic, but handle common status code
// maybe we can put the logic in requestHandler.ts in the future
// Now `throw new Response` or `throw new Error` is same, both will be caught by errorBoundary by default
// If loaderFailureMode is 'clientRender', we will downgrade to csr, and the loader will be request in client again
const errors = Object.values(
(routerContext.errors || {}) as Record<string, Error>,
);
if (
routerContext.statusCode >= 500 &&
routerContext.statusCode < 600 &&
// TODO: if loaderFailureMode is not 'errroBoundary', error log will not be printed.
errors.length > 0 &&
loaderFailureMode === 'clientRender'
) {
routerContext.statusCode = 200;
const errors = Object.values(
routerContext.errors as Record<string, Error>,
);
throw errors[0];
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,35 @@
export const loader = () => {
throw new Response(
JSON.stringify({
message: "can't found the user",
}),
{
status: 255,
},
);
import type { LoaderFunctionArgs } from '@modern-js/runtime/router';

export const loader = ({ request }: LoaderFunctionArgs) => {
const url = new URL(request.url);
const responseType = url.searchParams.get('type') as string;
const statusCode = url.searchParams.get('code') as string;

if (responseType === 'throw_error') {
throw new Error("can't found the user");
}

if (responseType === 'throw_response') {
throw new Response(
JSON.stringify({
message: "can't found the user",
}),
{
status: Number(statusCode),
},
);
}

if (responseType === 'return_response') {
return new Response(
JSON.stringify({
message: 'user modern.js',
}),
{
status: Number(statusCode),
},
);
}

return null;
};
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
import { Outlet, useLoaderData } from '@modern-js/runtime/router';
import { useLoaderData } from '@modern-js/runtime/router';

export default function Page() {
const data = useLoaderData();
return (
<div>
<Outlet />
</div>
);
return <div className="response-content">Response Page</div>;
}
94 changes: 83 additions & 11 deletions tests/integration/routes/tests/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -361,27 +361,67 @@ const supportLoader = async (page: Page, errors: string[], appPort: number) => {
expect(errors.length).toBe(0);
};

const supportThrowError = async (
page: Page,
errors: string[],
appPort: number,
) => {
const response = await page.goto(
`http://localhost:${appPort}/three/error/response?type=throw_error`,
{
waitUntil: ['domcontentloaded'],
},
);
expect(response?.status()).toBe(200);
await page.waitForSelector('.error-content');
const errorStatusElm = await page.$('.error-content');
const text = await page.evaluate(el => el?.textContent, errorStatusElm);
expect(text?.includes('500')).toBeFalsy();
expect(text?.includes("can't found the user")).toBeTruthy();
};

const supportThrowResponse = async (
page: Page,
errors: string[],
appPort: number,
code: number,
) => {
const response = await page.goto(
`http://localhost:${appPort}/three/error/response`,
`http://localhost:${appPort}/three/error/response?type=throw_response&code=${code}`,
{
waitUntil: ['domcontentloaded'],
},
);
expect(response?.status()).toBe(255);
expect(response?.status()).toBe(200);
await page.waitForSelector('.response-status');
const errorStatusElm = await page.$('.response-status');
const text = await page.evaluate(el => el?.textContent, errorStatusElm);
expect(text?.includes('255')).toBeTruthy();
expect(text?.includes(`${code}`)).toBeTruthy();
const errorContentElm = await page.$('.response-content');
const text1 = await page.evaluate(el => el?.textContent, errorContentElm);
expect(text1?.includes("can't found the user")).toBeTruthy();
expect(errors.length).toBe(0);
};

const supportReturnResponse = async (
page: Page,
errors: string[],
appPort: number,
code: number,
) => {
const response = await page.goto(
`http://localhost:${appPort}/three/error/response?type=return_response&code=${code}`,
{
waitUntil: ['domcontentloaded'],
},
);
expect(response?.status()).toBe(code);
await page.waitForSelector('.response-content');
const el = await page.$('.response-content');
const text = await page.evaluate(el => el?.textContent, el);
expect(text?.includes('Response Page')).toBeTruthy();
};

const supportLoaderForSSRAndCSR = async (
page: Page,
errors: string[],
Expand Down Expand Up @@ -770,8 +810,16 @@ describe('dev', () => {
supportRedirectForSSR(page, errors, appPort));
test('support redirect for csr', () =>
supportRedirectForCSR(page, errors, appPort));
test('support throw response', async () =>
supportThrowResponse(page, errors, appPort));
test('support throw error', async () =>
supportThrowError(page, errors, appPort));
test('support throw response', async () => {
await supportThrowResponse(page, errors, appPort, 500);
await supportThrowResponse(page, errors, appPort, 200);
});
test('support return response', async () => {
await supportReturnResponse(page, errors, appPort, 500);
await supportReturnResponse(page, errors, appPort, 200);
});
});

describe('global configuration', () => {
Expand Down Expand Up @@ -912,8 +960,16 @@ describe('build', () => {
supportRedirectForSSR(page, errors, appPort));
test('support redirect for csr', () =>
supportRedirectForCSR(page, errors, appPort));
test('support throw response', async () =>
supportThrowResponse(page, errors, appPort));
test('support throw error', async () =>
supportThrowError(page, errors, appPort));
test('support throw response', async () => {
await supportThrowResponse(page, errors, appPort, 500);
await supportThrowResponse(page, errors, appPort, 200);
});
test('support return response', async () => {
await supportReturnResponse(page, errors, appPort, 500);
await supportReturnResponse(page, errors, appPort, 200);
});
});

describe('global configuration', () => {
Expand Down Expand Up @@ -1056,8 +1112,16 @@ describe('dev with rspack', () => {
supportRedirectForSSR(page, errors, appPort));
test('support redirect for csr', () =>
supportRedirectForCSR(page, errors, appPort));
test('support throw response', async () =>
supportThrowResponse(page, errors, appPort));
test('support throw error', async () =>
supportThrowError(page, errors, appPort));
test('support throw response', async () => {
await supportThrowResponse(page, errors, appPort, 500);
await supportThrowResponse(page, errors, appPort, 200);
});
test('support return response', async () => {
await supportReturnResponse(page, errors, appPort, 500);
await supportReturnResponse(page, errors, appPort, 200);
});
});

describe('global configuration', () => {
Expand Down Expand Up @@ -1202,8 +1266,16 @@ describe('build with rspack', () => {
supportRedirectForSSR(page, errors, appPort));
test('support redirect for csr', () =>
supportRedirectForCSR(page, errors, appPort));
test('support throw response', async () =>
supportThrowResponse(page, errors, appPort));
test('support throw error', async () =>
supportThrowError(page, errors, appPort));
test('support throw response', async () => {
await supportThrowResponse(page, errors, appPort, 500);
await supportThrowResponse(page, errors, appPort, 200);
});
test('support return response', async () => {
await supportReturnResponse(page, errors, appPort, 500);
await supportReturnResponse(page, errors, appPort, 200);
});
});

describe('global configuration', () => {
Expand Down
Loading