Merge bitcoin/bitcoin#35384: util: Check write failures before renaming settings.json

0654511e1b util: Check write failures before renaming settings.json (Shrey)

Pull request description:

  This PR Fixes #35373.
  The error message was updated from: "This is probably caused by disk corruption or a crash" to: "This is probably caused by a full disk, disk corruption or a crash"

  The following files were updated:
  1. src/common/settings.cpp -- Updated the main string formatting for the parse failure.
  2. src/test/settings_tests.cpp -- Updated the exact string expectation in the C++ unit test.
  3. test/functional/feature_settings.py -- Updated the expected error message in the Python test suite.

  Configured the Bitcoin Core project natively on Windows using Visual Studio (cmake --preset vs2026-static -DBUILD_GUI=OFF) and it compiled with 100% of the C++ tests passed successfully (347 out of 347).

ACKs for top commit:
  maflcko:
    review ACK 0654511e1b 💧
  winterrdog:
    ACK 0654511e1b
  sedited:
    ACK 0654511e1b
  ryanofsky:
    Code review ACK 0654511e1b with commit message and error message improved since last review

Tree-SHA512: d7b49860f2081a5a9d1d44917b0cf372a77f10cf21773c9b3291871c788a122ec5dde063fdf9e1052cf45b8d2667e15585b952e9f037e84f98fb41546bdd1fa9
This commit is contained in:
Ryan Ofsky
2026-06-19 06:56:12 -04:00
3 changed files with 11 additions and 3 deletions

View File

@@ -86,7 +86,7 @@ bool ReadSettings(const fs::path& path, std::map<std::string, SettingsValue>& va
SettingsValue in;
if (!in.read(std::string{std::istreambuf_iterator<char>(file), std::istreambuf_iterator<char>()})) {
errors.emplace_back(strprintf("Settings file %s does not contain valid JSON. This is probably caused by disk corruption or a crash, "
errors.emplace_back(strprintf("Settings file %s does not contain valid JSON. This may be caused by a crash, power loss, full disk, or storage error, "
"and can be fixed by removing the file, which will reset settings to default values.",
fs::PathToString(path)));
return false;
@@ -139,7 +139,15 @@ bool WriteSettings(const fs::path& path,
return false;
}
file << out.write(/* prettyIndent= */ 4, /* indentLevel= */ 1) << std::endl;
if (file.fail()) {
errors.emplace_back(strprintf("Error: Unable to write settings file %s", fs::PathToString(path)));
return false;
}
file.close();
if (file.fail()) {
errors.emplace_back(strprintf("Error: Unable to close settings file %s", fs::PathToString(path)));
return false;
}
return true;
}

View File

@@ -101,7 +101,7 @@ BOOST_AUTO_TEST_CASE(ReadWrite)
// Check invalid json not allowed
WriteText(path, R"(invalid json)");
BOOST_CHECK(!common::ReadSettings(path, values, errors));
std::vector<std::string> fail_parse = {strprintf("Settings file %s does not contain valid JSON. This is probably caused by disk corruption or a crash, "
std::vector<std::string> fail_parse = {strprintf("Settings file %s does not contain valid JSON. This may be caused by a crash, power loss, full disk, or storage error, "
"and can be fixed by removing the file, which will reset settings to default values.",
fs::PathToString(path))};
BOOST_CHECK_EQUAL_COLLECTIONS(errors.begin(), errors.end(), fail_parse.begin(), fail_parse.end());

View File

@@ -73,7 +73,7 @@ class SettingsTest(BitcoinTestFramework):
# Test invalid json
with settings.open("w") as fp:
fp.write("invalid json")
node.assert_start_raises_init_error(expected_msg='does not contain valid JSON. This is probably caused by disk corruption or a crash', match=ErrorMatch.PARTIAL_REGEX)
node.assert_start_raises_init_error(expected_msg='does not contain valid JSON. This may be caused by a crash, power loss, full disk, or storage error', match=ErrorMatch.PARTIAL_REGEX)
# Test invalid json object
with settings.open("w") as fp: