-
-
Notifications
You must be signed in to change notification settings - Fork 405
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
update_manager: git repo validation is done on stale state, results in exceptions #820
Comments
Can you attach a log reproducing? The line numbers in the stack trace don't match the current repo. The error is indeed occuring because FWIW, the update manager isn't really intended to be used in this manner. Its primary purpose is to provide updates from a remote on a static branch. Manually modifying the repo could certainly confuse it. |
Wow, thanks for the super-quick response!
Sorry, I've been working with a codebase riddled with extra debug log statements. Here's a full log from unchanged code: Turns out, simply restarting is not sufficient to reproduce from clean state. I can get it to throw the exceptions when I force a refresh, but that alone doesn't save the broken state. Race condition maybe?
Sure, I get that, but it seems to have worked more cleanly in the past, i.e:
Now, no information about the actual problem shows up in Mainsail or fluidd - only the recovery button is highlighted and the info message remains stale:
It didn't occur to me that I'm doing anything unusual. I just had it running while working on some Klipper code in a branch. I know you only have so much time to work on this, so I'd be happy to fix this regression, unless you say "nah, I'm familiar with the codebase, I already know how to fix it". It's also worrying that the broken status was somehow saved and prevented a refresh even after switching back to Cheers |
BTW I still have one more corrupted repo state in storage, so I backed up the database in case it would help with debugging. |
I don't think this is a bug or a regression. Switching to a new branch that has an untracked remote would have still raised an exception prior to a7b9e57. Simply put, the update manager can't fetch the updates from a remote when there is no remote set in the git config. The way to correct the issue is to switch to a branch that is tracked, or push the branch to a remote. |
FWIW, i just verified. I was able to switch to a new branch on a configured repo and and reproduce the error. After switching back to The behavior from Moonraker is slightly different from a7b9e57, but both would have raised exceptions. In the current version the exception is raised when Moonraker attempts to verify the repo. The key In the linked commit the exception would have occurred here. In this version the call to One other thing I forgot to mention, the update manager's That said, Moonraker also registers the |
Hey, when I wrote that it was introduced in a7b9e57, I meant that the correct behavior is before it, meaning 35396a5. But it appears the behavior on startup in 35396a5 seems to be the same. What stands out is that in 35396a5 the function Still, the corruption of the saved state is freaky, but I cannot reproduce it at this time. Has to be a race condition of some sort. 🤔 |
Right, my mistake. As you stated, the behavior is the same.
That is true, however an exception in
Are you sure the saved state was corrupted? Mainsail will at times report corrupted when the repo is invalid. All that said, I did take a deeper look at this, and while it appears to me that its behaving as expected, the condition could be more clear to the user. In addition, the state should be saved on failure so its not reported as |
What happened
I noticed that for some repos updated with update_manager, I get the following stacktraces in logs:
On top of that, manually triggering the repo update in UI never seems to really refresh the repo state. I see warnings that are out-of-date.
After digging into the code, I determined the following:
GitDeploy.initialize()
, the repo state is restored from storageGitDeploy._update_repo_state()
wants to runGitRepo.refresh_repo_state()
and then verify the result withrepo.is_valid()
_update_repo_state()
just throws an exceptionmaster
?
_check_warnings()
for this exact condition:if self.git_branch != self.primary_branch:
refresh_repo_state
would only run this waaay after the exception is thrown_check_warnings
is ran is that initial check based on stale staterepo.report_invalids()
Client
Fluidd
Browser
Firefox
How to reproduce
Additional information
No response
The text was updated successfully, but these errors were encountered: