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.

Sign up for GitHub

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

Jump to bottom

Clicking on an organization dashboard during a warehouse migration will report a 500 error #20804

Closed
suiyueou opened this issue Aug 16, 2022 · 3 comments · Fixed by #20840 or #20842
Closed

Clicking on an organization dashboard during a warehouse migration will report a 500 error #20804

suiyueou opened this issue Aug 16, 2022 · 3 comments · Fixed by #20840 or #20842
Labels
type/bug
Milestone
1.17.1

Comments

@suiyueou
Copy link

suiyueou commented Aug 16, 2022 •
edited
Loading

Description

When an organization is performing a repository migration task in the background, if you switch to the corresponding organization's dashboard in a new tab at this time, a 500 error is reported

In testing, I found that only the organization's dashboard had this problem, everything was fine for the user

Gitea Version

1.17.0

Can you reproduce the bug on the Gitea demo site?

No

Log Gist

No response

Screenshots

image

Git Version

2.36.2

Operating System

Ubuntu 22.04 LTS

How are you running Gitea?

Docker Deploy

Database

SQLite
@lunny
Copy link
Member

lunny commented Aug 16, 2022

When migrating a mirror, the mirror record hasn't been inserted into database at the beginning.

@suiyueou
Copy link
Author

When migrating a mirror, the mirror record hasn't been inserted into database at the beginning.

It is when the organization is executing the repository migration task in the background, at this time the user accesses the link /org/orgName/dashboard/ will report 500 error, this error I also tested on the old version (1.16.3), and there is no, should be the new version introduced.

For example, a mirror organization is migrating a gitea repository in the background, and during the migration, when I visit the /org/mirror/dashboard page, I get a 500 error.

This error is only triggered when the organization is performing a migration task, and when the migration task is complete, the error disappears and access returns to normal

@lunny lunny added this to the 1.17.1 milestone Aug 16, 2022
zeripath added a commit to zeripath/gitea that referenced this issue Aug 17, 2022
@zeripath
Check Mirror exists before linking its Repo
9bacd90 
In MirrorRepositoryList.loadAttributes there is some code to load the Mirror entries
from the database. This assumes that every Repository which has IsMirror set has
a Mirror associated in the DB. This association is incorrect in the case of
Mirror repository under creation when there is no Mirror entry in the DB until
completion.

Unfortunately LoadAttributes makes this incorrect assumption and presumes that a
Mirror will always be loaded. This then causes a panic.

This PR simply double checks if there a Mirror before attempting to link back to
its Repo. Unfortunately it should be expected that there may be other cases where
this incorrect assumption causes further problems.

Fix go-gitea#20804

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath mentioned this issue Aug 17, 2022
Merged
@zeripath
Copy link
Contributor

I've put up a PR to prevent the panic in this code but I think we need to look again at this code and consider whether it's even necessary to call it as I'm not sure that the value of .Mirrors is ever used...

lunny added a commit that referenced this issue Aug 18, 2022
@zeripath @lunny
Check Mirror exists before linking its Repo (#20840)
03df7d0 
In MirrorRepositoryList.loadAttributes there is some code to load the Mirror entries
from the database. This assumes that every Repository which has IsMirror set has
a Mirror associated in the DB. This association is incorrect in the case of
Mirror repository under creation when there is no Mirror entry in the DB until
completion.

Unfortunately LoadAttributes makes this incorrect assumption and presumes that a
Mirror will always be loaded. This then causes a panic.

This PR simply double checks if there a Mirror before attempting to link back to
its Repo. Unfortunately it should be expected that there may be other cases where
this incorrect assumption causes further problems.

Fix #20804

Signed-off-by: Andrew Thornton <art27@cantab.net>

Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
lunny added a commit to lunny/gitea that referenced this issue Aug 18, 2022
@zeripath @lunny
Check Mirror exists before linking its Repo (go-gitea#20840)
cc347b1 
In MirrorRepositoryList.loadAttributes there is some code to load the Mirror entries
from the database. This assumes that every Repository which has IsMirror set has
a Mirror associated in the DB. This association is incorrect in the case of
Mirror repository under creation when there is no Mirror entry in the DB until
completion.

Unfortunately LoadAttributes makes this incorrect assumption and presumes that a
Mirror will always be loaded. This then causes a panic.

This PR simply double checks if there a Mirror before attempting to link back to
its Repo. Unfortunately it should be expected that there may be other cases where
this incorrect assumption causes further problems.

Fix go-gitea#20804

Signed-off-by: Andrew Thornton <art27@cantab.net>

Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
@lunny lunny mentioned this issue Aug 18, 2022
Merged
@zeripath zeripath linked a pull request Aug 18, 2022 that will close this issue
Check Mirror exists before linking its Repo (#20840) #20842
Merged
lafriks pushed a commit that referenced this issue Aug 18, 2022
@lunny @zeripath
Check Mirror exists before linking its Repo (#20840) (#20842)
c40c753 
In MirrorRepositoryList.loadAttributes there is some code to load the Mirror entries
from the database. This assumes that every Repository which has IsMirror set has
a Mirror associated in the DB. This association is incorrect in the case of
Mirror repository under creation when there is no Mirror entry in the DB until
completion.

Unfortunately LoadAttributes makes this incorrect assumption and presumes that a
Mirror will always be loaded. This then causes a panic.

This PR simply double checks if there a Mirror before attempting to link back to
its Repo. Unfortunately it should be expected that there may be other cases where
this incorrect assumption causes further problems.

Fix #20804

Signed-off-by: Andrew Thornton <art27@cantab.net>

Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>

Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: zeripath <art27@cantab.net>
zeripath added a commit to zeripath/gitea that referenced this issue Aug 18, 2022
@zeripath
Remove calls to load Mirrors in user.Dashboard
6226355 
Whilst looking at go-gitea#20840 I noticed that the Mirrors data doesn't appear
to be being used therefore we can remove this and in fact none of the
related code is used elsewhere so it can also be removed.

Related go-gitea#20840
Related go-gitea#20804

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath mentioned this issue Aug 18, 2022
Merged
techknowlogick pushed a commit that referenced this issue Aug 19, 2022
@zeripath
Remove calls to load Mirrors in user.Dashboard (#20855)
68f3aae 
Whilst looking at #20840 I noticed that the Mirrors data doesn't appear
to be being used therefore we can remove this and in fact none of the
related code is used elsewhere so it can also be removed.

Related #20840
Related #20804

Signed-off-by: Andrew Thornton <art27@cantab.net>

Signed-off-by: Andrew Thornton <art27@cantab.net>
zeripath added a commit to zeripath/gitea that referenced this issue Aug 21, 2022
@zeripath
Remove calls to load Mirrors in user.Dashboard (go-gitea#20855)
9192e3d 
Backport go-gitea#20855

Whilst looking at go-gitea#20840 I noticed that the Mirrors data doesn't appear
to be being used therefore we can remove this and in fact none of the
related code is used elsewhere so it can also be removed.

Related go-gitea#20840
Related go-gitea#20804

Signed-off-by: Andrew Thornton <art27@cantab.net>

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath mentioned this issue Aug 21, 2022
Merged
lunny added a commit that referenced this issue Aug 22, 2022
@zeripath @lunny
Remove calls to load Mirrors in user.Dashboard (#20855) (#20897)
ec9b43b 
Backport #20855

Whilst looking at #20840 I noticed that the Mirrors data doesn't appear
to be being used therefore we can remove this and in fact none of the
related code is used elsewhere so it can also be removed.

Related #20840
Related #20804

Signed-off-by: Andrew Thornton <art27@cantab.net>

Signed-off-by: Andrew Thornton <art27@cantab.net>

Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
vsysoev pushed a commit to IntegraSDL/gitea that referenced this issue Aug 28, 2022
@zeripath @lunny
Check Mirror exists before linking its Repo (go-gitea#20840)
1eb6443 
In MirrorRepositoryList.loadAttributes there is some code to load the Mirror entries
from the database. This assumes that every Repository which has IsMirror set has
a Mirror associated in the DB. This association is incorrect in the case of
Mirror repository under creation when there is no Mirror entry in the DB until
completion.

Unfortunately LoadAttributes makes this incorrect assumption and presumes that a
Mirror will always be loaded. This then causes a panic.

This PR simply double checks if there a Mirror before attempting to link back to
its Repo. Unfortunately it should be expected that there may be other cases where
this incorrect assumption causes further problems.

Fix go-gitea#20804

Signed-off-by: Andrew Thornton <art27@cantab.net>

Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
vsysoev pushed a commit to IntegraSDL/gitea that referenced this issue Aug 28, 2022
@zeripath
Remove calls to load Mirrors in user.Dashboard (go-gitea#20855)
1c3b60e 
Whilst looking at go-gitea#20840 I noticed that the Mirrors data doesn't appear
to be being used therefore we can remove this and in fact none of the
related code is used elsewhere so it can also be removed.

Related go-gitea#20840
Related go-gitea#20804

Signed-off-by: Andrew Thornton <art27@cantab.net>

Signed-off-by: Andrew Thornton <art27@cantab.net>
@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.
Assignees
No one assigned
Labels
type/bug
Projects
None yet
Milestone
1.17.1
3 participants
@lunny @zeripath @suiyueou