FEATURE: Catch decorateCooked errors from themes/plugins (#15450)

If a theme/plugin raises an error while decorating post content, the decorator will be skipped, and the error reported on the console. Additionally, administrators will be shown a red warning at the top of the screen.

This commit refactors and re-uses some of the logic from the theme-initializer-error-reporting logic. In future, new error reports can be added by doing something like:

```
document.dispatchEvent(
  new CustomEvent("discourse-error", {
    detail: { messageKey: "some.translation.key", error },
  })
);
```
This commit is contained in:
David Taylor 2022-01-04 21:59:52 +00:00 committed by GitHub
parent 94560d2383
commit 1f1aa6a0d8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 104 additions and 40 deletions

View File

@ -3,7 +3,7 @@ import { buildResolver } from "discourse-common/resolver";
import { isTesting } from "discourse-common/config/environment"; import { isTesting } from "discourse-common/config/environment";
const _pluginCallbacks = []; const _pluginCallbacks = [];
let _themeErrors = []; let _unhandledThemeErrors = [];
const Discourse = Application.extend({ const Discourse = Application.extend({
rootElement: "#main", rootElement: "#main",
@ -24,12 +24,11 @@ const Discourse = Application.extend({
if (!module) { if (!module) {
throw new Error(moduleName + " must export an initializer."); throw new Error(moduleName + " must export an initializer.");
} }
} catch (err) { } catch (error) {
if (!themeId || isTesting()) { if (!themeId || isTesting()) {
throw err; throw error;
} }
_themeErrors.push([themeId, err]); fireThemeErrorEvent({ themeId, error });
fireThemeErrorEvent();
return; return;
} }
@ -38,12 +37,11 @@ const Discourse = Application.extend({
init.initialize = (app) => { init.initialize = (app) => {
try { try {
return oldInitialize.call(init, app.__container__, app); return oldInitialize.call(init, app.__container__, app);
} catch (err) { } catch (error) {
if (!themeId || isTesting()) { if (!themeId || isTesting()) {
throw err; throw error;
} }
_themeErrors.push([themeId, err]); fireThemeErrorEvent({ themeId, error });
fireThemeErrorEvent();
} }
}; };
@ -92,14 +90,22 @@ function moduleThemeId(moduleName) {
} }
} }
function fireThemeErrorEvent() { function fireThemeErrorEvent({ themeId, error }) {
const event = new CustomEvent("discourse-theme-error"); const event = new CustomEvent("discourse-error", {
document.dispatchEvent(event); cancelable: true,
detail: { themeId, error },
});
const unhandled = document.dispatchEvent(event);
if (unhandled) {
_unhandledThemeErrors.push(event);
}
} }
export function getAndClearThemeErrors() { export function getAndClearUnhandledThemeErrors() {
const copy = _themeErrors; const copy = _unhandledThemeErrors;
_themeErrors = []; _unhandledThemeErrors = [];
return copy; return copy;
} }

View File

@ -118,6 +118,21 @@ function canModify(klass, type, resolverName, changes) {
} }
} }
function wrapWithErrorHandler(func, messageKey) {
return function () {
try {
return func.call(this, ...arguments);
} catch (error) {
document.dispatchEvent(
new CustomEvent("discourse-error", {
detail: { messageKey, error },
})
);
return;
}
};
}
class PluginApi { class PluginApi {
constructor(version, container) { constructor(version, container) {
this.version = version; this.version = version;
@ -309,6 +324,8 @@ class PluginApi {
decorateCookedElement(callback, opts) { decorateCookedElement(callback, opts) {
opts = opts || {}; opts = opts || {};
callback = wrapWithErrorHandler(callback, "broken_decorator_alert");
addDecorator(callback, { afterAdopt: !!opts.afterAdopt }); addDecorator(callback, { afterAdopt: !!opts.afterAdopt });
if (!opts.onlyStream) { if (!opts.onlyStream) {

View File

@ -1,22 +1,43 @@
import { isTesting } from "discourse-common/config/environment"; import { isTesting } from "discourse-common/config/environment";
import { getAndClearThemeErrors } from "discourse/app"; import { getAndClearUnhandledThemeErrors } from "discourse/app";
import PreloadStore from "discourse/lib/preload-store"; import PreloadStore from "discourse/lib/preload-store";
import getURL from "discourse-common/lib/get-url"; import getURL from "discourse-common/lib/get-url";
import I18n from "I18n"; import I18n from "I18n";
import { bind } from "discourse-common/utils/decorators";
const showingErrors = new Set();
export default { export default {
name: "theme-errors-handler", name: "theme-errors-handler",
after: "inject-discourse-objects", after: "inject-discourse-objects",
initialize(container) { initialize(container) {
const currentUser = container.lookup("current-user:main");
if (isTesting()) { if (isTesting()) {
return; return;
} }
renderErrorNotices(currentUser);
document.addEventListener("discourse-theme-error", () => this.currentUser = container.lookup("current-user:main");
renderErrorNotices(currentUser)
); getAndClearUnhandledThemeErrors().forEach((e) => {
reportThemeError(this.currentUser, e);
});
document.addEventListener("discourse-error", this.handleDiscourseError);
},
cleanup() {
document.removeEventListener(this.handleDiscourseError);
delete this.currentUser;
},
@bind
handleDiscourseError(e) {
if (e.detail?.themeId) {
reportThemeError(this.currentUser, e);
} else {
reportGenericError(this.currentUser, e);
}
e.preventDefault(); // Mark as handled
}, },
}; };
@ -32,24 +53,42 @@ function reportToLogster(name, error) {
}); });
} }
function renderErrorNotices(currentUser) { function reportThemeError(currentUser, e) {
getAndClearThemeErrors().forEach(([themeId, error]) => { const { themeId, error } = e.detail;
const name = const name =
PreloadStore.get("activatedThemes")[themeId] || `(theme-id: ${themeId})`; PreloadStore.get("activatedThemes")[themeId] || `(theme-id: ${themeId})`;
/* eslint-disable-next-line no-console */ /* eslint-disable-next-line no-console */
console.error(`An error occurred in the "${name}" theme/component:`, error); console.error(`An error occurred in the "${name}" theme/component:`, error);
reportToLogster(name, error); reportToLogster(name, error);
if (!currentUser || !currentUser.admin) {
return;
}
const path = getURL("/admin/customize/themes"); const path = getURL("/admin/customize/themes");
const message = I18n.t("themes.broken_theme_alert", { const message = I18n.t("themes.broken_theme_alert", {
theme: name, theme: name,
path: `<a href="${path}">${path}</a>`, path: `<a href="${path}">${path}</a>`,
}); });
displayErrorNotice(currentUser, message);
}
function reportGenericError(currentUser, e) {
const { messageKey, error } = e.detail;
/* eslint-disable-next-line no-console */
console.error(error);
if (messageKey && !showingErrors.has(messageKey)) {
showingErrors.add(messageKey);
displayErrorNotice(currentUser, I18n.t(messageKey));
}
}
function displayErrorNotice(currentUser, message) {
if (!currentUser?.admin) {
return;
}
const alertDiv = document.createElement("div"); const alertDiv = document.createElement("div");
alertDiv.classList.add("broken-theme-alert"); alertDiv.classList.add("broken-theme-alert");
alertDiv.innerHTML = `⚠️ ${message}`; alertDiv.innerHTML = `⚠️ ${message}`;
document.body.prepend(alertDiv); document.body.prepend(alertDiv);
});
} }

View File

@ -195,6 +195,8 @@ en:
default_description: "Default" default_description: "Default"
broken_theme_alert: "Your site may not work because theme / component %{theme} has errors. Disable it at %{path}." broken_theme_alert: "Your site may not work because theme / component %{theme} has errors. Disable it at %{path}."
broken_decorator_alert: "Posts may not display correctly because one of the post content decorators on your site raised an error. Check the browser developer tools for more information."
s3: s3:
regions: regions:
ap_northeast_1: "Asia Pacific (Tokyo)" ap_northeast_1: "Asia Pacific (Tokyo)"