jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/395039 )
Change subject: Fix: show original server errors
......................................................................
Fix: show original server errors
When the server encounters an error, show the original error encountered
on the server instead of allowing a client side render which may cause a
different error.
Bug: T181900
Change-Id: I3603e26ffdbb8f0313c71c8c598055639b085a54
---
M package.json
M src/client/index.tsx
A src/common/models/ssr-data.ts
M src/common/pages/error.tsx
M src/common/pages/not-found.tsx
M src/common/router/router.ts
M src/server/components/html-page.tsx
M src/server/index.tsx
M test/server/components/html-page.test.ts
9 files changed, 56 insertions(+), 14 deletions(-)
Approvals:
Jhernandez: Looks good to me, approved
jenkins-bot: Verified
diff --git a/package.json b/package.json
index 4bef766..88be03a 100644
--- a/package.json
+++ b/package.json
@@ -104,7 +104,7 @@
"bundlesize": [
{
"path": "dist/public/index.*.js",
- "maxSize": "3.7KB"
+ "maxSize": "3.8KB"
},
{
"path": "dist/public/runtime.*.js",
diff --git a/src/client/index.tsx b/src/client/index.tsx
index 0386958..3ded82a 100644
--- a/src/client/index.tsx
+++ b/src/client/index.tsx
@@ -3,6 +3,7 @@
import "wikimedia-ui-base/wikimedia-ui-base.css";
import "./index.css";
import { RouteResponse, newRouter } from "../common/router/router";
+import { SSRData } from "../common/models/ssr-data";
import { WithContext } from "../common/components/with-context";
import { routes } from "../common/router/routes";
@@ -13,6 +14,7 @@
require("preact/debug");
}
+const ssrData: SSRData = (window as any).__SSR_DATA__;
const history = newHistory();
const router = newRouter(routes);
const pageRoot = (_ => {
@@ -43,4 +45,6 @@
// Replace the server rendered root, which does not include CSS, with a styled
// page that manages navigation with History. This enables the single page app
// experience.
-route(location.pathname);
+if (!ssrData.forceSSR) {
+ route(location.pathname);
+}
diff --git a/src/common/models/ssr-data.ts b/src/common/models/ssr-data.ts
new file mode 100644
index 0000000..a881d5b
--- /dev/null
+++ b/src/common/models/ssr-data.ts
@@ -0,0 +1,9 @@
+/**
+ * Server-side rendered data. This data is serialized by the server and
+ * deserialized by the client through the `window.__SSR_DATA__` global which is
+ * set prior to any client script execution.
+ */
+export interface SSRData {
+ /** Forbid client-side rendering of subsequently requested pages. */
+ forceSSR: boolean;
+}
diff --git a/src/common/pages/error.tsx b/src/common/pages/error.tsx
index 1a13979..6c1bbe2 100644
--- a/src/common/pages/error.tsx
+++ b/src/common/pages/error.tsx
@@ -1,25 +1,25 @@
import { h } from "preact";
-import App from "../components/app/app";
import Page from "../components/page/page";
export interface Props {
error: Error;
}
+// Note: this page should not require any client-side JavaScript. It is not
+// re-rendered on the client and so does not allow interactions such as the app
+// menu.
export default {
status: 500,
Component({ error }: Props): JSX.Element {
return (
- <App>
- <Page title={`Unexpected error: ${error.message}`} subtitle="Error
500">
- <p>
- <pre>
- <code>{error.stack}</code>
- </pre>
- </p>
- </Page>
- </App>
+ <Page title={`Unexpected error: ${error.message}`} subtitle="Error 500">
+ <p>
+ <pre>
+ <code>{error.stack}</code>
+ </pre>
+ </p>
+ </Page>
);
}
};
diff --git a/src/common/pages/not-found.tsx b/src/common/pages/not-found.tsx
index 16564a6..ef7fc98 100644
--- a/src/common/pages/not-found.tsx
+++ b/src/common/pages/not-found.tsx
@@ -6,6 +6,10 @@
path: string;
}
+// Note: visually, this page may have some similarities to the generic
+// unexpected error page. However, this module should be kept distinct because
+// it permits interaction and may be rendered client-side, unlike the error
+// generic error page.
export default {
status: 404,
diff --git a/src/common/router/router.ts b/src/common/router/router.ts
index 6030598..0e5f69f 100644
--- a/src/common/router/router.ts
+++ b/src/common/router/router.ts
@@ -54,6 +54,8 @@
);
}
+// todo: can we load this page dynamically instead? Many users will never even
+// see a 404.
function respondNotFound(path: string): Promise<RouteResponse<any>> {
const props: NotFoundProps = { path };
return Promise.resolve({
diff --git a/src/server/components/html-page.tsx
b/src/server/components/html-page.tsx
index 61603ff..12fcdb6 100644
--- a/src/server/components/html-page.tsx
+++ b/src/server/components/html-page.tsx
@@ -2,6 +2,7 @@
import { PRODUCTION } from "../../common/assets/config";
import { asset } from "../../common/assets/manifest";
import { ChildrenProps } from "../../common/components/preact-utils";
+import { SSRData } from "../../common/models/ssr-data";
declare function __non_webpack_require__(name: string): any; //
eslint-disable-line camelcase
export interface Props extends ChildrenProps {
@@ -10,6 +11,7 @@
// Chunk to preload on the HTML. May not be needed if the chunks are already
// included, like the error pages
chunkName?: string;
+ ssrData: SSRData;
}
// The production asset manifest from the public build products or
@@ -23,6 +25,7 @@
export default function HTMLPage({
title = "",
chunkName,
+ ssrData,
children
}: Props): JSX.Element {
// Asset order matters.
@@ -36,6 +39,10 @@
const style = asset("index", "css", manifest);
const favicon = asset("favicon/wikipedia", "ico", manifest);
+ // Serialize the SSR data safely. See:
+ //
https://github.com/reactjs/redux/blob/cda8699/docs/recipes/ServerRendering.md#inject-initial-component-html-and-state
+ const ssrDataString = JSON.stringify(ssrData).replace(/</g, "\\u003c");
+
return (
<html lang="en">
<head>
@@ -46,6 +53,13 @@
{/* Preload the stylesheet before the scripts */}
<link rel="preload" href={style} as="style" />
<link rel="stylesheet" href={style} />
+ {/* Set the global server-side rendered data before scripts execute.
*/}
+ <script
+ type="text/javascript"
+ dangerouslySetInnerHTML={{
+ __html: `window.__SSR_DATA__ = ${ssrDataString}`
+ }}
+ />
{assets.map(path => <link rel="preload" href={path} as="script" />)}
<link rel="shortcut icon" href={favicon} />
</head>
diff --git a/src/server/index.tsx b/src/server/index.tsx
index 2d63976..efcc6c8 100644
--- a/src/server/index.tsx
+++ b/src/server/index.tsx
@@ -6,6 +6,7 @@
import { RedirectError } from "../common/http/fetch";
import { routes } from "../common/router/routes";
import { SERVER_PORT, SERVER_URL } from "../common/assets/config";
+import ErrorPage from "../common/pages/error";
import HTMLPage from "./components/html-page";
const server = express();
@@ -19,10 +20,13 @@
server.use("/public", express.static("dist/public"));
function render({ chunkName, Component, props }: RouteResponse<any>) {
+ // When an unexpected error occurs, forbid client re-rendering so that the
+ // original error can be shown.
+ const forceSSR = Component === ErrorPage.Component;
return (
"<!doctype html>" + // eslint-disable-line prefer-template
renderToString(
- <HTMLPage title="" chunkName={chunkName}>
+ <HTMLPage title="" chunkName={chunkName} ssrData={{ forceSSR }}>
<Component {...props} />
</HTMLPage>
)
diff --git a/test/server/components/html-page.test.ts
b/test/server/components/html-page.test.ts
index a9b1243..ef48709 100644
--- a/test/server/components/html-page.test.ts
+++ b/test/server/components/html-page.test.ts
@@ -7,6 +7,7 @@
const vNode = HTMLPage({
title: "Test",
chunkName: "",
+ ssrData: { forceSSR: false },
children: ["body"]
});
const html = render(vNode);
@@ -18,7 +19,11 @@
});
it("contains a <title/> when rendered", () => {
- const vNode = HTMLPage({ title: "Test", chunkName: "" });
+ const vNode = HTMLPage({
+ title: "Test",
+ chunkName: "",
+ ssrData: { forceSSR: false }
+ });
const html = render(vNode);
const expected = "<title>Test - Marvin</title>";
assert.ok(
--
To view, visit https://gerrit.wikimedia.org/r/395039
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I3603e26ffdbb8f0313c71c8c598055639b085a54
Gerrit-PatchSet: 8
Gerrit-Project: marvin
Gerrit-Branch: master
Gerrit-Owner: Niedzielski <[email protected]>
Gerrit-Reviewer: Jhernandez <[email protected]>
Gerrit-Reviewer: Niedzielski <[email protected]>
Gerrit-Reviewer: Sniedzielski <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
