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

Nicer error handling #1024

Merged
merged 38 commits into from
May 23, 2023
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
f31cd42
Redirect fomr pages that require auth on logout
SleeplessOne1917 May 13, 2023
ab3fed3
Extract helper function
SleeplessOne1917 May 14, 2023
bcee6aa
Set up logic for handling errors
SleeplessOne1917 May 14, 2023
d944140
Merge branch 'main' into nicer-error-hnadling
SleeplessOne1917 May 14, 2023
8f2d964
Fix server redirect error
SleeplessOne1917 May 14, 2023
76f0292
Redirect to login and remove duplicated code
SleeplessOne1917 May 14, 2023
025daaa
Use node env instead of version for environment specific logic
SleeplessOne1917 May 14, 2023
3996cda
Handle error when site not returned
SleeplessOne1917 May 14, 2023
23d7751
Fix error page not showing when site not fetched and adjust styles
SleeplessOne1917 May 15, 2023
dbee253
Fix things not working in production build
SleeplessOne1917 May 15, 2023
5068df6
Merge branch 'main' into nicer-error-hnadling
SleeplessOne1917 May 16, 2023
900bf20
Cleanup
SleeplessOne1917 May 16, 2023
24c4427
Get rid or forced error
SleeplessOne1917 May 16, 2023
4f1d357
Refactor how error data is passed from server to client
SleeplessOne1917 May 17, 2023
256420b
Fix isoData bug
SleeplessOne1917 May 17, 2023
7241abc
Merge branch 'main' into nicer-error-hnadling
SleeplessOne1917 May 17, 2023
75d2fb8
Replace link to issue tracker with proper support spaces
SleeplessOne1917 May 19, 2023
cbee588
Merge branch 'nicer-error-hnadling' of https://github.com/SleeplessOn…
SleeplessOne1917 May 19, 2023
e542aa8
Incorporate translations
SleeplessOne1917 May 21, 2023
a8e2b43
Remove console logs
SleeplessOne1917 May 21, 2023
117e85a
Include forgotten translation
SleeplessOne1917 May 21, 2023
8d0b2e0
Merge branch 'main' into nicer-error-hnadling
SleeplessOne1917 May 21, 2023
044cc88
Merge branch 'main' into nicer-error-hnadling
SleeplessOne1917 May 22, 2023
fe69f8e
Merge branch 'main' into nicer-error-hnadling
SleeplessOne1917 May 22, 2023
83fc9f3
Make error code always display
SleeplessOne1917 May 22, 2023
1f54933
Merge branch 'nicer-error-hnadling' of https://github.com/SleeplessOn…
SleeplessOne1917 May 22, 2023
51c0c98
Add error message paragraph
SleeplessOne1917 May 22, 2023
498de66
User HTTP instead of HTTPS when fetching icon in docker internal network
SleeplessOne1917 May 22, 2023
7fbb12a
Add debug statement.
SleeplessOne1917 May 22, 2023
fd39b21
Revert "Add debug statement."
SleeplessOne1917 May 22, 2023
19408b2
Revert "User HTTP instead of HTTPS when fetching icon in docker inter…
SleeplessOne1917 May 22, 2023
ee95bd0
Always replace host with internal host
SleeplessOne1917 May 22, 2023
5af62eb
Merge branch 'main' into nicer-error-hnadling
SleeplessOne1917 May 23, 2023
c311bca
Merge branch 'main' into nicer-error-hnadling
SleeplessOne1917 May 23, 2023
696c5b0
Run prettier
SleeplessOne1917 May 23, 2023
7030eb8
Merge branch 'nicer-error-hnadling' of https://github.com/SleeplessOn…
SleeplessOne1917 May 23, 2023
06a553c
Hopefully stop lint command from erroring
SleeplessOne1917 May 23, 2023
65fc023
Merge branch 'main' into nicer-error-hnadling
SleeplessOne1917 May 23, 2023
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
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
"isomorphic-cookie": "^1.2.4",
"jwt-decode": "^3.1.2",
"lemmy-js-client": "0.17.2-rc.17",
"lodash": "^4.17.21",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lodash seems unused, and I'd def like to avoid it if possible.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lodash is only being used in webpack.config.js. I originally tried having it as a dev dependency, but this caused issues when I tested the prod Dockerfile. This shouldn't be included in the bundle.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah i got this issue too when trying to run docker compose against lemmy-ui

"markdown-it": "^13.0.1",
"markdown-it-container": "^3.0.0",
"markdown-it-emoji": "^2.0.2",
Expand All @@ -77,6 +78,7 @@
"sass": "^1.62.1",
"sass-loader": "^13.2.2",
"serialize-javascript": "^6.0.1",
"service-worker-webpack": "^1.0.0",
"sharp": "^0.32.1",
"tippy.js": "^6.3.7",
"toastify-js": "^1.12.0",
Expand Down Expand Up @@ -111,7 +113,6 @@
"prettier-plugin-organize-imports": "^3.2.2",
"prettier-plugin-packagejson": "^2.4.3",
"rimraf": "^5.0.0",
"service-worker-webpack": "^1.0.0",
"sortpack": "^2.3.4",
"style-loader": "^3.3.2",
"terser": "^5.17.3",
Expand Down
303 changes: 174 additions & 129 deletions src/server/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,16 @@ import { getHttpBase, getHttpBaseInternal } from "../shared/env";
import {
ILemmyConfig,
InitialFetchRequest,
IsoData,
IsoDataOptionalSite,
} from "../shared/interfaces";
import { routes } from "../shared/routes";
import { favIconPngUrl, favIconUrl, initializeSite } from "../shared/utils";
import {
ErrorPageData,
favIconPngUrl,
favIconUrl,
initializeSite,
isAuthPath,
} from "../shared/utils";

const server = express();
const [hostname, port] = process.env["LEMMY_UI_HOST"]
Expand Down Expand Up @@ -70,6 +76,7 @@ server.get("/css/themes/:name", async (req, res) => {
res.contentType("text/css");
const theme = req.params.name;
if (!theme.endsWith(".css")) {
res.statusCode = 400;
res.send("Theme must be a css file");
}

Expand Down Expand Up @@ -109,44 +116,58 @@ server.get("/css/themelist", async (_req, res) => {
server.get("/*", async (req, res) => {
try {
const activeRoute = routes.find(route => matchPath(req.path, route));
const context = {} as any;
let auth: string | undefined = IsomorphicCookie.load("jwt", req);

const getSiteForm: GetSite = { auth };

const promises: Promise<any>[] = [];

const headers = setForwardedHeaders(req.headers);
const client = new LemmyHttp(getHttpBaseInternal(), headers);

const { path, url, query } = req;

// Get site data first
// This bypasses errors, so that the client can hit the error on its own,
// in order to remove the jwt on the browser. Necessary for wrong jwts
let try_site: any = await client.getSite(getSiteForm);
if (try_site.error == "not_logged_in") {
console.error(
"Incorrect JWT token, skipping auth so frontend can remove jwt cookie"
);
getSiteForm.auth = undefined;
auth = undefined;
try_site = await client.getSite(getSiteForm);
}
const site: GetSiteResponse = try_site;
initializeSite(site);

const initialFetchReq: InitialFetchRequest = {
client,
auth,
path: req.path,
query: req.query,
site,
};
let site: GetSiteResponse | undefined = undefined;
let routeData: any[] = [];
let errorPageData: ErrorPageData | undefined;
try {
let try_site: any = await client.getSite(getSiteForm);
if (try_site.error == "not_logged_in") {
console.error(
"Incorrect JWT token, skipping auth so frontend can remove jwt cookie"
);
getSiteForm.auth = undefined;
auth = undefined;
try_site = await client.getSite(getSiteForm);
}

if (activeRoute?.fetchInitialData) {
promises.push(...activeRoute.fetchInitialData(initialFetchReq));
}
if (!auth && isAuthPath(path)) {
res.redirect("/login");
return;
}

const routeData = await Promise.all(promises);
site = try_site;
initializeSite(site);

if (site) {
const initialFetchReq: InitialFetchRequest = {
client,
auth,
path,
query,
site,
};

if (activeRoute?.fetchInitialData) {
routeData = await Promise.all([
...activeRoute.fetchInitialData(initialFetchReq),
]);
}
}
} catch (error) {
errorPageData = getErrorPageData(error, site);
}

// Redirect to the 404 if there's an API error
if (routeData[0] && routeData[0].error) {
Expand All @@ -155,112 +176,33 @@ server.get("/*", async (req, res) => {
if (error === "instance_is_private") {
return res.redirect(`/signup`);
} else {
return res.send(`404: ${removeAuthParam(error)}`);
errorPageData = getErrorPageData(error, site);
}
}

const isoData: IsoData = {
path: req.path,
const isoData: IsoDataOptionalSite = {
path,
site_res: site,
routeData,
errorPageData,
};

const wrapper = (
<StaticRouter location={req.url} context={isoData}>
<StaticRouter location={url} context={isoData}>
<App />
</StaticRouter>
);
if (context.url) {
return res.redirect(context.url);
}

const eruda = (
<>
<script src="//cdn.jsdelivr.net/npm/eruda"></script>
<script>eruda.init();</script>
</>
);

const erudaStr = process.env["LEMMY_UI_DEBUG"] ? renderToString(eruda) : "";
const root = renderToString(wrapper);
const helmet = Helmet.renderStatic();

const config: ILemmyConfig = { wsHost: process.env.LEMMY_UI_LEMMY_WS_HOST };

const appleTouchIcon = site.site_view.site.icon
? `data:image/png;base64,${sharp(
await fetchIconPng(site.site_view.site.icon)
)
.resize(180, 180)
.extend({
bottom: 20,
top: 20,
left: 20,
right: 20,
background: "#222222",
})
.png()
.toBuffer()
.then(buf => buf.toString("base64"))}`
: favIconPngUrl;

res.send(`
<!DOCTYPE html>
<html ${helmet.htmlAttributes.toString()} lang="en">
<head>
<script>window.isoData = ${JSON.stringify(isoData)}</script>
<script>window.lemmyConfig = ${serialize(config)}</script>

<!-- A remote debugging utility for mobile -->
${erudaStr}

<!-- Custom injected script -->
${customHtmlHeader}

${helmet.title.toString()}
${helmet.meta.toString()}

<!-- Required meta tags -->
<meta name="Description" content="Lemmy">
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1, shrink-to-fit=no">
<link
id="favicon"
rel="shortcut icon"
type="image/x-icon"
href=${site.site_view.site.icon ?? favIconUrl}
/>

<!-- Web app manifest -->
<link rel="manifest" href="data:application/manifest+json;base64,${await generateManifestBase64(
site.site_view.site
)}">
<link rel="apple-touch-icon" href=${appleTouchIcon} />
<link rel="apple-touch-startup-image" href=${appleTouchIcon} />

<!-- Styles -->
<link rel="stylesheet" type="text/css" href="/static/styles/styles.css" />

<!-- Current theme and more -->
${helmet.link.toString()}

</head>

<body ${helmet.bodyAttributes.toString()}>
<noscript>
<div class="alert alert-danger rounded-0" role="alert">
<b>Javascript is disabled. Actions will not work.</b>
</div>
</noscript>

<div id='root'>${root}</div>
<script defer src='/static/js/client.js'></script>
</body>
</html>
`);

res.send(await createSsrHtml(root, isoData));
} catch (err) {
// If an error is caught here, the error page couldn't even be rendered
console.error(err);
return res.send(`404: ${removeAuthParam(err)}`);
res.statusCode = 500;
return res.send(
process.env.NODE_ENV === "development" ? err.message : "Server error"
);
}
});

Expand Down Expand Up @@ -292,16 +234,6 @@ process.on("SIGINT", () => {
process.exit(0);
});

function removeAuthParam(err: any): string {
return removeParam(err.toString(), "auth");
}

function removeParam(url: string, parameter: string): string {
return url
.replace(new RegExp("[?&]" + parameter + "=[^&#]*(#.*)?$"), "$1")
.replace(new RegExp("([?&])" + parameter + "=[^&]*&"), "$1");
}

const iconSizes = [72, 96, 128, 144, 152, 192, 384, 512];
const defaultLogoPathDirectory = path.join(
process.cwd(),
Expand Down Expand Up @@ -361,3 +293,116 @@ async function fetchIconPng(iconUrl: string) {
.then(res => res.blob())
.then(blob => blob.arrayBuffer());
}

function getErrorPageData(error: string, site?: GetSiteResponse) {
const errorPageData: ErrorPageData = {};

// Exact error should only be seen in a development environment. Users
// in production will get a more generic message.
if (process.env.NODE_ENV === "development") {
errorPageData.error = error;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im not sure this makes sense. The reason not to show error messages is usually for security, but anyway the error messages are available if you use the API directly.

Another reason might be to keep things simple for non-techie users. But even if they dont understand the error, there can be a text telling them to copy-paste the error message to admins/devs. That can be very helpful for debugging.


const adminMatrixIds = site?.admins
.map(({ person: { matrix_user_id } }) => matrix_user_id)
.filter(id => id) as string[] | undefined;
if (adminMatrixIds && adminMatrixIds.length > 0) {
errorPageData.adminMatrixIds = adminMatrixIds;
}

return errorPageData;
}

async function createSsrHtml(root: string, isoData: IsoDataOptionalSite) {
const site = isoData.site_res;
const appleTouchIcon = site?.site_view.site.icon
? `data:image/png;base64,${sharp(
await fetchIconPng(site.site_view.site.icon)
)
.resize(180, 180)
.extend({
bottom: 20,
top: 20,
left: 20,
right: 20,
background: "#222222",
})
.png()
.toBuffer()
.then(buf => buf.toString("base64"))}`
: favIconPngUrl;

const eruda = (
<>
<script src="//cdn.jsdelivr.net/npm/eruda"></script>
<script>eruda.init();</script>
</>
);

const erudaStr = process.env["LEMMY_UI_DEBUG"] ? renderToString(eruda) : "";

const helmet = Helmet.renderStatic();

const config: ILemmyConfig = { wsHost: process.env.LEMMY_UI_LEMMY_WS_HOST };

return `
<!DOCTYPE html>
<html ${helmet.htmlAttributes.toString()} lang="en">
<head>
<script>window.isoData = ${JSON.stringify(isoData)}</script>
<script>window.lemmyConfig = ${serialize(config)}</script>

<!-- A remote debugging utility for mobile -->
${erudaStr}

<!-- Custom injected script -->
${customHtmlHeader}

${helmet.title.toString()}
${helmet.meta.toString()}

<!-- Required meta tags -->
<meta name="Description" content="Lemmy">
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1, shrink-to-fit=no">
<link
id="favicon"
rel="shortcut icon"
type="image/x-icon"
href=${site?.site_view.site.icon ?? favIconUrl}
/>

<!-- Web app manifest -->
${
site &&
`<link
rel="manifest"
href={${`data:application/manifest+json;base64,${await generateManifestBase64(
site.site_view.site
)}`}}
/>`
}
<link rel="apple-touch-icon" href=${appleTouchIcon} />
<link rel="apple-touch-startup-image" href=${appleTouchIcon} />

<!-- Styles -->
<link rel="stylesheet" type="text/css" href="/static/styles/styles.css" />

<!-- Current theme and more -->
${helmet.link.toString()}

</head>

<body ${helmet.bodyAttributes.toString()}>
<noscript>
<div class="alert alert-danger rounded-0" role="alert">
<b>Javascript is disabled. Actions will not work.</b>
</div>
</noscript>

<div id='root'>${root}</div>
<script defer src='/static/js/client.js'></script>
</body>
</html>
`;
}
Loading