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

Simplify the error message when index.js couldn't be loaded #22354

Merged
merged 8 commits into from
Jan 18, 2023

Conversation

wxiaoguang
Copy link
Contributor

In some cases, the loading failure of index.js is not related to the ROOT_URL directly, ex: https://gitea.com/gitea/helm-chart/issues/392

If the user's reversed proxy is mis-configured: http://public-domain/gitea/xxx -> http://gitea:3000/gitea/xxx, it also causes the loading failure.

So this PR removes the ROOT_URL related tip from the error message.

@wxiaoguang wxiaoguang added the skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. label Jan 6, 2023
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Jan 6, 2023
templates/base/footer.tmpl Outdated Show resolved Hide resolved
@@ -22,7 +22,7 @@
<script src='https://hcaptcha.com/1/api.js' async></script>
{{end}}
{{end}}
<script src="{{AssetUrlPrefix}}/js/index.js?v={{AssetVersion}}" 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>
Copy link
Member

@delvh delvh Jan 6, 2023

Choose a reason for hiding this comment

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

But wasn't this the exact reason why you added this alert, especially the ROOT_URL, in the first part?
Given that it is still often enough the reason, I think we should keep it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, this is not. See the original PR.

Copy link
Member

Choose a reason for hiding this comment

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

Isn't {{AssetUrlPrefix}} built from ROOT_URL ? Or some other app.ini setting ? It would be useful to be hinted about which app.ini setting affects that url...

Copy link
Contributor Author

@wxiaoguang wxiaoguang Jan 12, 2023

Choose a reason for hiding this comment

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

Isn't {{AssetUrlPrefix}} built from ROOT_URL ?

No, not directly.

Or some other app.ini setting ?

Somewhat, but not directly. See the PR's comment: ex: https://gitea.com/gitea/helm-chart/issues/392

It would be useful to be hinted about which app.ini setting affects that url...

There are many different cases. In most cases, users could fix it by themselves. And in most cases, the problem is caused by user's environment (the example in the PR's comment). If not, it's better to collect common problems and write a FAQ.

templates/base/footer.tmpl Outdated Show resolved Hide resolved
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jan 16, 2023
@codecov-commenter
Copy link

codecov-commenter commented Jan 16, 2023

Codecov Report

Merging #22354 (f6e6f7e) into main (d9f748a) will increase coverage by 48.14%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           main   #22354       +/-   ##
=========================================
+ Coverage      0   48.14%   +48.14%     
=========================================
  Files         0     1054     +1054     
  Lines         0   143450   +143450     
=========================================
+ Hits          0    69059    +69059     
- Misses        0    66117    +66117     
- Partials      0     8274     +8274     
Impacted Files Coverage Δ
models/user/external_login_user.go 27.27% <0.00%> (ø)
services/auth/source/oauth2/urlmapping.go 24.32% <0.00%> (ø)
models/project/board.go 29.67% <0.00%> (ø)
services/auth/source/pam/source.go 25.00% <0.00%> (ø)
services/convert/mirror.go 0.00% <0.00%> (ø)
modules/util/timer.go 100.00% <0.00%> (ø)
models/asymkey/ssh_key_authorized_keys.go 16.15% <0.00%> (ø)
modules/nosql/leveldb.go 66.66% <0.00%> (ø)
routers/api/v1/user/app.go 66.97% <0.00%> (ø)
models/db/log.go 40.81% <0.00%> (ø)
... and 1044 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jan 17, 2023
@jolheiser jolheiser merged commit 7ddc11d into go-gitea:main Jan 18, 2023
@jolheiser jolheiser removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jan 18, 2023
@wxiaoguang wxiaoguang deleted the wxiaoguang-patch-1 branch January 18, 2023 06:15
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jan 18, 2023
* upstream/main:
  Simplify the error message when `index.js` couldn't be loaded (go-gitea#22354)
  Support asciicast files as new markup (go-gitea#22448)
  Support scoped access tokens (go-gitea#20908)
  some refactor about code comments (go-gitea#20821)
  docs: add swagger.json file location to FAQ (go-gitea#22489)
  docs: bump Gitea version (go-gitea#22490)
  chore: changelog 1.18.1 (go-gitea#22471) (go-gitea#22487)
  Fixed lint warnings in Grafana raised by Mixtool (go-gitea#22486)
  Set disable_gravatar/enable_federated_avatar when offline mode is true (go-gitea#22479)
  Fix pull request API field `closed_at` always being `null` (go-gitea#22482)
  Fix migration from gitbucket (repost) (go-gitea#22477)
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants