-
Notifications
You must be signed in to change notification settings - Fork 3
fix: handle corrupted config files gracefully instead of crashing #116
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
Conversation
- Use default values when config options are missing instead of throwing - Regenerate config automatically when options are missing or malformed - Add TRUNCATE_EXISTING to prevent leftover data from corrupting config - Remove System.exit(1) on config parse errors to allow recovery Fixes imthosea#115
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.
Pull request overview
This PR addresses issue #115 where corrupted config files cause the application to crash. The fix aims to handle missing or malformed config options gracefully by using default values and automatically regenerating the config file instead of terminating the application.
Key Changes:
- Removed
System.exit(1)call when config loading fails, allowing the application to recover - Added
hasMissingOptionsflag to track when config regeneration is needed - Changed error handling to log warnings and use defaults for missing options instead of throwing exceptions
- Added
TRUNCATE_EXISTINGflag towriteConfig()to prevent leftover data corruption
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| ConfigLoadContext.java | Adds missing options tracking flag, removes System.exit on load failure, and changes str() method to return null for missing options instead of throwing |
| Config.java | Updates config regeneration logic to trigger on missing options and adds TRUNCATE_EXISTING to prevent partial writes |
Comments suppressed due to low confidence (1)
common/src/main/java/me/thosea/badoptimizations/config/Config.java:90
- System.exit(1) should also be removed here to be consistent with the fix in loadProp(). If writing the config fails after handling a corrupted config, the application will still crash. Consider logging the error and continuing with default values in memory, or retrying the write operation.
if(!ctx.fromExistingFile() || ver < CURRENT_CONFIG_VER || ctx.hasMissingOptions) {
try {
writeConfig();
} catch(Exception e) {
LOGGER.error("Failed to write default config to " + FILE, e);
System.exit(1);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if(str == null) { | ||
| throw new IllegalStateException("Config option " + name + " not found"); | ||
| LOGGER.warn("Config option {} not found - using default value. Config will be regenerated.", name); | ||
| hasMissingOptions = true; | ||
| } |
Copilot
AI
Dec 25, 2025
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.
While this handles missing config options gracefully, it doesn't address the complete issue described in #115. The config corruption in the issue shows malformed values (incomplete strings like "enable_sky_color_cach" instead of "enable_sky_color_caching"). When such malformed values are parsed by boolOrDefault or intOrDefault, they will still throw IllegalStateException and crash. Consider also setting hasMissingOptions = true in those parsing methods when they encounter malformed values, rather than throwing exceptions.
| public final ModIncompatibilities incompats = new ModIncompatibilities(); | ||
| public final int version; | ||
| @Nullable public final Properties properties; | ||
| public boolean hasMissingOptions = false; |
Copilot
AI
Dec 25, 2025
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.
The name 'hasMissingOptions' suggests it only tracks missing config options, but conceptually it should also track malformed options that trigger regeneration. Consider renaming to 'needsRegeneration' or 'hasCorruptedOptions' to better reflect its purpose, or update the implementation to handle both missing and malformed values.
Fixes #115