Skip to content

Conversation

lunny
Copy link
Member

@lunny lunny commented Dec 5, 2020

The recent change to recovery middleware will always output trace log content to web browser when panic. It's a regression.

This PR introduced some new middlewares to chi and will fix that and take them back.

  • Introduce new render middleware from https://github.com/unrolled/render.
  • Introduce new locale middleware which could get the current language.
  • Session middleware has been copied and modified to https://gitea.com/go-chi/session.
  • Login user panic page should know user information.
  • code.gitea.io/gitea/modules/auth/sso has been refactored so that both macaron and chi could use that.
  • Improve the Recovery middleware, now it will try to render a friendly UI at first, if panic again, fallback to original method

@lunny lunny added type/bug skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. labels Dec 5, 2020
@lunny lunny changed the title Fix recovery middleware to render gitea style page. WIP: Fix recovery middleware to render gitea style page. Dec 5, 2020
@lunny lunny force-pushed the lunny/fix_recovery branch 5 times, most recently from d9cc999 to 6c60d40 Compare December 19, 2020 07:50
@lunny lunny changed the title WIP: Fix recovery middleware to render gitea style page. Fix recovery middleware to render gitea style page. Dec 19, 2020
@codecov-io
Copy link

codecov-io commented Dec 19, 2020

Codecov Report

Merging #13857 (b793642) into master (126c933) will decrease coverage by 0.09%.
The diff coverage is 26.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #13857      +/-   ##
==========================================
- Coverage   41.97%   41.87%   -0.10%     
==========================================
  Files         735      742       +7     
  Lines       78933    79309     +376     
==========================================
+ Hits        33132    33211      +79     
- Misses      40346    40635     +289     
- Partials     5455     5463       +8     
Impacted Files Coverage Δ
modules/auth/auth.go 61.53% <ø> (-2.51%) ⬇️
modules/auth/sso/reverseproxy.go 9.52% <0.00%> (ø)
modules/middlewares/cookie.go 0.00% <0.00%> (ø)
modules/middlewares/locale.go 0.00% <0.00%> (ø)
modules/middlewares/redis.go 2.15% <2.15%> (ø)
modules/templates/base.go 28.57% <28.57%> (ø)
routers/routes/recovery.go 29.41% <29.41%> (ø)
modules/middlewares/virtual.go 32.29% <32.29%> (ø)
modules/auth/sso/basic.go 51.06% <50.00%> (ø)
routers/init.go 45.79% <50.00%> (-4.61%) ⬇️
... and 21 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 126c933...b793642. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Dec 19, 2020
@lunny lunny added this to the 1.14.0 milestone Dec 19, 2020
@lunny lunny force-pushed the lunny/fix_recovery branch from c7a09e3 to d28e7dd Compare December 20, 2020 05:07
@lunny lunny requested review from zeripath and 6543 December 20, 2020 05:08
@lunny lunny force-pushed the lunny/fix_recovery branch from 7d42691 to bc8fbde Compare December 21, 2020 13:56
@lunny
Copy link
Member Author

lunny commented Dec 21, 2020

@6543 All done.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Dec 21, 2020
@lunny lunny force-pushed the lunny/fix_recovery branch from ec5a9f6 to df74647 Compare December 21, 2020 23:43
@lunny
Copy link
Member Author

lunny commented Dec 22, 2020

@techknowlogick Could you review this again?

@lunny lunny force-pushed the lunny/fix_recovery branch from df74647 to f1daa29 Compare December 22, 2020 11:29
@lunny lunny force-pushed the lunny/fix_recovery branch from 39d4ce3 to c178867 Compare December 24, 2020 04:30
@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 5, 2021
@6543
Copy link
Member

6543 commented Jan 5, 2021

🎉

@lunny lunny merged commit 15a475b into go-gitea:master Jan 5, 2021
@lunny lunny deleted the lunny/fix_recovery branch January 5, 2021 13:05
@lunny lunny mentioned this pull request Jan 5, 2021
@CirnoT CirnoT mentioned this pull request Jan 7, 2021
CirnoT added a commit to CirnoT/gitea that referenced this pull request Jan 7, 2021
@go-gitea go-gitea locked and limited conversation to collaborators Feb 11, 2021
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. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants