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

Better recovery when registry.json is corrupted #3351

Merged
merged 2 commits into from
May 6, 2021

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Apr 17, 2021

Problem

If your CKAN/registry.json file gets truncated somehow, CKAN won't start.

Unhandled exception:
Newtonsoft.Json.JsonSerializationException: Unexpected end when deserializing object. Path 'available_modules.WindowShineTR.module_version['1:2.0'].download_hash.sha256', line 979045, position 0.
Unhandled exception:
Newtonsoft.Json.JsonReaderException: Unexpected end of content while loading JArray. Path 'available_modules.RP-0.module_version['v1.6'].recommends[21]', line 648536, position 8403726.
   in Newtonsoft.Json.Linq.JContainer.ReadTokenFrom(JsonReader reader, JsonLoadSettings options)
   in Newtonsoft.Json.Linq.JArray.Load(JsonReader reader, JsonLoadSettings settings)
   in Newtonsoft.Json.Linq.JToken.ReadFrom(JsonReader reader, JsonLoadSettings settings)
Newtonsoft.Json.JsonReaderException: Unterminated string. Expected delimiter: ". Path 'available_modules.WarpShip.module_version['0.3.2'].resources.repository', line 878236, position 44.

Examples of this happening in the wild:

#1386, #1540, #1904, #1779, #2123, #2448, #2566, #2588, #2871, #3188, #3229, #3233

(Inspired by #3347.)

Cause

The deserialization call can throw JsonSerializationException or JsonReaderException (both inherit from JsonException), and we don't handle it.

We currently don't have any clues as to how or why the corruption happens, because the exception can be thrown long after the file is corrupted. By the time the user knows it happened, it's too late to investigate.

Changes

Now if our attempt to load registry.json throws a JsonException, RegistryManager.LoadOrCreate will catch it and move the file to a new location, CKAN/registry.json_CORRUPTED_20210417020300, where that last part is the current date. Then a new registry will be created in its place, to be filled in as usual for a new registry. This way even though the user's data is lost, they will still have a working CKAN, and they may be able to recover some data from the corrupted file. Then the GUI notifies the user that this happened.

@HebaruSan HebaruSan added Bug Something is not working as intended Easy This is easy to fix Core (ckan.dll) Issues affecting the core part of CKAN Pull request Registry Issues affecting the registry labels Apr 17, 2021
@HebaruSan
Copy link
Member Author

We currently don't have any clues as to how or why it happens, because the exception can be thrown long after the file is corrupted. By the time the user knows it happened, it's too late to investigate.

It's been a while since it's happened, I wonder if #3250 helped.

@DasSkelett
Copy link
Member

Hm, could we set some sort of (unserialized) flag in the Registry object when it is detected as corrupted and moved, which we later check when the GUI is ready, and display a warning?
Hacky, but better than no note to the user at all, IMHO.

@HebaruSan
Copy link
Member Author

Hm, could we set some sort of (unserialized) flag in the Registry object when it is detected as corrupted and moved, which we later check when the GUI is ready, and display a warning?

Done. I liked RegistryManager better as a place for this flag since it is not really related to the active registry object itself, and the RegistryManager is the class that discovers the problem in the first place.

* Check for registry corruption after switching game instances
* Clear corrupted registry message after displaying it
* Print more detailed error message to help with recovery
* German translation for error message
@DasSkelett
Copy link
Member

Pushed some small changes, now we also check if a corruption happened (or better: got detected) during instance switch.
The error message also contains a hint towards the installed-<instanceName>.ckan file to help with recovery, for that I moved some string calculations around to make them more accessible.

Copy link
Member

@DasSkelett DasSkelett left a comment

Choose a reason for hiding this comment

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

Looks good. Probably a long overdue feature, saves some "complicated" manual steps that were inevitable anyways.

Could we automate the reinstallation of all the previously installed modules next...?

@HebaruSan HebaruSan merged commit 72c4a56 into KSP-CKAN:master May 6, 2021
@HebaruSan HebaruSan deleted the fix/corrupt-json-recovery branch May 6, 2021 23:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is not working as intended Core (ckan.dll) Issues affecting the core part of CKAN Easy This is easy to fix Registry Issues affecting the registry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants