-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Refetch local metadata cache if corruption is detected #31509
base: master
Are you sure you want to change the base?
Conversation
Addresses one of the points in ppy#31496. Not going to lie, this is mostly best-effort stuff (while the refetch is happening, metadata lookups using the local source *will* fail), but I see this as a marginal scenario anyways.
Seems to work well. I've applied some style changes, please check you're okay with the changes @bdach . |
if (cacheDownloadRequest != null) | ||
throw; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a behavioural change? I think this should return false
? If the intention was to rethrow to the catch (Exception ex)
block below then I'm not sure that's how this works.
In an isolated console project, when I do something like
try
{
throw new InvalidOperationException();
}
catch (InvalidOperationException)
{
throw;
}
catch
{
Console.WriteLine("caught");
}
then the resulting program is not printing to the console, it's throwing the error:
Unhandled exception. System.InvalidOperationException: Operation is not valid due to the current state of the object.
at Program.<Main>$(String[] args) in /home/dachb/Documents/Projekty/ConsoleApp1/ConsoleApp1/Program.cs:line 5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, that's unfortunate. i thought that's how it worked but i guess not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made one more attempt.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems better now yeah.
Addresses one of the points in #31496.
Not going to lie, this is mostly best-effort stuff (while the refetch is happening, metadata lookups using the local source will fail), but I see this as a marginal scenario anyways.