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

Show messages for users if the ROOT_URL is wrong, show JavaScript errors #18971

Merged
merged 15 commits into from
Mar 30, 2022
Merged
Show file tree
Hide file tree
Changes from 8 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
2 changes: 1 addition & 1 deletion templates/base/footer.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
<script src='https://hcaptcha.com/1/api.js' async></script>
{{end}}
{{end}}
<script src="{{AssetUrlPrefix}}/js/index.js?v={{MD5 AppVer}}"></script>
<script src="{{AssetUrlPrefix}}/js/index.js?v={{MD5 AppVer}}" onerror="alert('Failed to load asset files from ' + this.src + ', please make sure the asset files can be accessed and the ROOT_URL setting in app.ini is correct.')"></script>
{{template "custom/footer" .}}
</body>
</html>
45 changes: 3 additions & 42 deletions templates/base/head.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -19,51 +19,12 @@
<link rel="alternate" type="application/atom+xml" title="" href="{{.FeedURL}}.atom">
<link rel="alternate" type="application/rss+xml" title="" href="{{.FeedURL}}.rss">
{{end}}
<script>
<!-- /* eslint-disable */ -->
window.config = {
appVer: '{{AppVer}}',
appSubUrl: '{{AppSubUrl}}',
assetUrlPrefix: '{{AssetUrlPrefix}}',
runModeIsProd: {{.RunModeIsProd}},
customEmojis: {{CustomEmojis}},
useServiceWorker: {{UseServiceWorker}},
csrfToken: '{{.CsrfToken}}',
pageData: {{.PageData}},
requireTribute: {{.RequireTribute}},
notificationSettings: {{NotificationSettings}}, {{/*a map provided by NewFuncMap in helper.go*/}}
enableTimeTracking: {{EnableTimetracking}},
{{if .RequireTribute}}
tributeValues: Array.from(new Map([
{{ range .Participants }}
['{{.Name}}', {key: '{{.Name}} {{.FullName}}', value: '{{.Name}}',
name: '{{.Name}}', fullname: '{{.FullName}}', avatar: '{{.AvatarLink}}'}],
{{ end }}
{{ range .Assignees }}
['{{.Name}}', {key: '{{.Name}} {{.FullName}}', value: '{{.Name}}',
name: '{{.Name}}', fullname: '{{.FullName}}', avatar: '{{.AvatarLink}}'}],
{{ end }}
{{ range .MentionableTeams }}
['{{$.MentionableTeamsOrg}}/{{.Name}}', {key: '{{$.MentionableTeamsOrg}}/{{.Name}}', value: '{{$.MentionableTeamsOrg}}/{{.Name}}',
name: '{{$.MentionableTeamsOrg}}/{{.Name}}', avatar: '{{$.MentionableTeamsOrgAvatar}}'}],
{{ end }}
]).values()),
{{end}}
mermaidMaxSourceCharacters: {{MermaidMaxSourceCharacters}},
{{/* this global i18n object should only contain general texts. for specialized texts, it should be provided inside the related modules by: (1) API response (2) HTML data-attribute (3) PageData */}}
i18n: {
copy_success: '{{.i18n.Tr "copy_success"}}',
copy_error: '{{.i18n.Tr "copy_error"}}',
error_occurred: '{{.i18n.Tr "error.occurred"}}',
network_error: '{{.i18n.Tr "error.network_error"}}',
},
};
{{/* in case some pages don't render the pageData, we make sure it is an object to prevent null access */}}
window.config.pageData = window.config.pageData || {};
</script>
<link rel="icon" href="{{AssetUrlPrefix}}/img/logo.svg" type="image/svg+xml">
<link rel="alternate icon" href="{{AssetUrlPrefix}}/img/favicon.png" type="image/png">
<link rel="stylesheet" href="{{AssetUrlPrefix}}/css/index.css?v={{MD5 AppVer}}">

{{template "base/head_script" .}}

<noscript>
<style>
.dropdown:hover > .menu { display: block; }
Expand Down
44 changes: 44 additions & 0 deletions templates/base/head_script.tmpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
<script>
<!-- /* eslint-disable */ -->
window.addEventListener('error', function(e) {window._globalHandlerErrors=window._globalHandlerErrors||[]; window._globalHandlerErrors.push(e);});
Copy link
Member

Choose a reason for hiding this comment

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

Should probably create a separate webpack chunk that loads beforeindex.js for this instead of inlining it. That way, you also shouldn't need this global variable.

Copy link
Contributor Author

@wxiaoguang wxiaoguang Mar 29, 2022

Choose a reason for hiding this comment

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

However, index.js is very late, the layout is:

window.addEventListener('error')
window.config =  // error occurs 
template <script> // error occurs
index.js 
- bootstrap.js (the old publicpath.js)
- others (initXxx)  // error occurs

So the window.addEventListener must be the first one in script tag.

window.config = {
appVer: '{{AppVer}}',
appUrl: '{{AppUrl}}',
appSubUrl: '{{AppSubUrl}}',
assetUrlPrefix: '{{AssetUrlPrefix}}',
runModeIsProd: {{.RunModeIsProd}},
customEmojis: {{CustomEmojis}},
useServiceWorker: {{UseServiceWorker}},
csrfToken: '{{.CsrfToken}}',
pageData: {{.PageData}},
requireTribute: {{.RequireTribute}},
notificationSettings: {{NotificationSettings}}, {{/*a map provided by NewFuncMap in helper.go*/}}
enableTimeTracking: {{EnableTimetracking}},
{{if .RequireTribute}}
tributeValues: Array.from(new Map([
{{ range .Participants }}
['{{.Name}}', {key: '{{.Name}} {{.FullName}}', value: '{{.Name}}',
name: '{{.Name}}', fullname: '{{.FullName}}', avatar: '{{.AvatarLink}}'}],
{{ end }}
{{ range .Assignees }}
['{{.Name}}', {key: '{{.Name}} {{.FullName}}', value: '{{.Name}}',
name: '{{.Name}}', fullname: '{{.FullName}}', avatar: '{{.AvatarLink}}'}],
{{ end }}
{{ range .MentionableTeams }}
['{{$.MentionableTeamsOrg}}/{{.Name}}', {key: '{{$.MentionableTeamsOrg}}/{{.Name}}', value: '{{$.MentionableTeamsOrg}}/{{.Name}}',
name: '{{$.MentionableTeamsOrg}}/{{.Name}}', avatar: '{{$.MentionableTeamsOrgAvatar}}'}],
{{ end }}
]).values()),
{{end}}
mermaidMaxSourceCharacters: {{MermaidMaxSourceCharacters}},
{{/* this global i18n object should only contain general texts. for specialized texts, it should be provided inside the related modules by: (1) API response (2) HTML data-attribute (3) PageData */}}
i18n: {
copy_success: '{{.i18n.Tr "copy_success"}}',
copy_error: '{{.i18n.Tr "copy_error"}}',
error_occurred: '{{.i18n.Tr "error.occurred"}}',
network_error: '{{.i18n.Tr "error.network_error"}}',
},
};
{{/* in case some pages don't render the pageData, we make sure it is an object to prevent null access */}}
window.config.pageData = window.config.pageData || {};
</script>
40 changes: 40 additions & 0 deletions web_src/js/bootstrap.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import {joinPaths} from './utils.js';

// DO NOT IMPORT window.config HERE!
// to make sure the error handler always works, we should never import `window.config`, because some user's custom template breaks it.

// This sets up the URL prefix used in webpack's chunk loading.
// This file must be imported before any lazy-loading is being attempted.
__webpack_public_path__ = joinPaths(window?.config?.assetUrlPrefix ?? '/', '/');

export function showGlobalErrorMessageHtml(msgHtml) {
const pageContent = document.querySelector('.page-content');
if (!pageContent) return;
const el = document.createElement('div');
el.innerHTML = `<div class="ui container negative message center aligned js-global-error">${msgHtml}</div>`;
pageContent.prepend(el.childNodes[0]);
}

/**
* @param {ErrorEvent} e
*/
function processWindowErrorEvent(e) {
showGlobalErrorMessageHtml(`JavaScript error: ${e.message} (${e.filename} @ ${e.lineno}:${e.colno}). Open browser console to see more details.`);
}

function initGlobalErrorHandler() {
if (!window.config) {
showGlobalErrorMessageHtml(`Gitea JavaScript code couldn't run correctly, please check your custom templates`);
}

// we added an event handler for window error at the very beginning of head.tmpl
// the handler calls `_globalHandlerErrors.push` (array method) to record all errors occur before this init
// then in this init, we can collect all error events and show them
for (const e of window._globalHandlerErrors || []) {
processWindowErrorEvent(e);
}
// then, change _globalHandlerErrors to an object with push method, to process further error events directly
window._globalHandlerErrors = {'push': (e) => processWindowErrorEvent(e)};
}

initGlobalErrorHandler();
23 changes: 22 additions & 1 deletion web_src/js/features/common-global.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ import 'jquery.are-you-sure';
import {mqBinarySearch} from '../utils.js';
import createDropzone from './dropzone.js';
import {initCompColorPicker} from './comp/ColorPicker.js';
import {htmlEscape} from 'escape-goat';
import {showGlobalErrorMessageHtml} from '../bootstrap.js';

const {csrfToken} = window.config;
const {appUrl, csrfToken} = window.config;

export function initGlobalFormDirtyLeaveConfirm() {
// Warn users that try to leave a page after entering data into a form.
Expand Down Expand Up @@ -343,3 +345,22 @@ export function initGlobalButtons() {
});
});
}

/**
* Too many users set their ROOT_URL to wrong value, and it causes a lot of problems:
* * Cross-origin API request without correct cookie
* * Incorrect href in <a>
* * ...
* So we check whether current URL starts with AppUrl(ROOT_URL).
* If they don't match, show a warning to users.
*/
export function checkAppUrl() {
const curUrl = window.location.href;
if (curUrl.startsWith(appUrl)) {
return;
}
showGlobalErrorMessageHtml(`
Your ROOT_URL in app.ini is ${htmlEscape(appUrl)} but you are visiting ${htmlEscape(curUrl)}<br />
You should set ROOT_URL correctly, otherwise the web may not work correctly.
`);
}
7 changes: 5 additions & 2 deletions web_src/js/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import './publicpath.js';
// bootstrap module must be the first one to be imported, it handles webpack lazy-loading and global errors
import './bootstrap.js';

import $ from 'jquery';
import {initVueEnv} from './components/VueComponentLoader.js';
Expand Down Expand Up @@ -39,6 +40,7 @@ import {
} from './features/repo-issue.js';
import {initRepoEllipsisButton, initRepoCommitLastCommitLoader} from './features/repo-commit.js';
import {
checkAppUrl,
initFootLanguageMenu,
initGlobalButtonClickOnEnter,
initGlobalButtons,
Expand Down Expand Up @@ -82,7 +84,6 @@ $.fn.tab.settings.silent = true;
$.fn.checkbox.settings.enableEnterKey = false;

initVueEnv();

$(document).ready(() => {
initGlobalCommon();

Expand Down Expand Up @@ -169,4 +170,6 @@ $(document).ready(() => {
initUserAuthWebAuthn();
initUserAuthWebAuthnRegister();
initUserSettings();

checkAppUrl();
});
6 changes: 0 additions & 6 deletions web_src/js/publicpath.js

This file was deleted.