diff --git a/src/microReticulum/Provisioning/Provisioning.cpp b/src/microReticulum/Provisioning/Provisioning.cpp index 25818f09..9263e5e8 100644 --- a/src/microReticulum/Provisioning/Provisioning.cpp +++ b/src/microReticulum/Provisioning/Provisioning.cpp @@ -775,6 +775,7 @@ namespace RNS { namespace Provisioning { size_t applied_total = 0; bool any_reboot = false; + bool storage_ok = true; auto do_one = [&](Namespace& ns) { // Pre-commit hook: fires once per namespace iff at least one @@ -804,7 +805,12 @@ namespace RNS { namespace Provisioning { default: break; } } - if (_storage) _storage->save_namespace(ns); + // A commit is only successful if the promoted working state made it + // to durable storage. Previously this return value was discarded, so + // wire clients received a normal Commit response even when the + // namespace file could not be written. The in-process commit() path + // already propagates this failure; keep the wire path consistent. + if (_storage && !_storage->save_namespace(ns)) storage_ok = false; }; if (has_filter) { @@ -817,6 +823,14 @@ namespace RNS { namespace Provisioning { for (const auto& ns_ptr : _registry.namespaces()) do_one(*ns_ptr); } set_reboot_flag(any_reboot); + if (!storage_ok) { + return encode_error( + (opid_t)Op::Commit, + seq, + ErrorCode::StorageError, + "failed to persist committed state" + ); + } return pack_response((opid_t)Op::Commit, seq, [&](MsgPack::Packer& p) { p.serialize(MsgPack::map_size_t(2)); diff --git a/src/microReticulum/Reticulum.cpp b/src/microReticulum/Reticulum.cpp index 190ac635..d0e21a5a 100644 --- a/src/microReticulum/Reticulum.cpp +++ b/src/microReticulum/Reticulum.cpp @@ -530,7 +530,11 @@ void Reticulum::get_packet_q(const Bytes& packet_hash) const { Bytes buf; if (OS::read_file(time_offset_path, buf) == 8) { uint64_t offset = *(uint64_t*)buf.data(); - DEBUGF("Read time offset of %llu from file", offset); + DEBUGF( + "Read time offset 0x%08lx%08lx from file", + (unsigned long)(offset >> 32), + (unsigned long)(offset & 0xFFFFFFFFULL) + ); OS::setTimeOffset(offset); return true; } @@ -550,7 +554,12 @@ void Reticulum::get_packet_q(const Bytes& packet_hash) const { char time_offset_path[FILEPATH_MAXSIZE]; snprintf(time_offset_path, FILEPATH_MAXSIZE, "%s/time_offset", _storagepath); uint64_t offset = OS::ltime(); - TRACEF("Writing time offset of %llu to file %s", offset, time_offset_path); + TRACEF( + "Writing time offset 0x%08lx%08lx to file %s", + (unsigned long)(offset >> 32), + (unsigned long)(offset & 0xFFFFFFFFULL), + time_offset_path + ); Bytes buf((uint8_t*)&offset, sizeof(offset)); OS::write_file(time_offset_path, buf); return true; diff --git a/test/test_provisioning/test_provisioning.cpp b/test/test_provisioning/test_provisioning.cpp index 5cb4dfd8..b1aa8b9f 100644 --- a/test/test_provisioning/test_provisioning.cpp +++ b/test/test_provisioning/test_provisioning.cpp @@ -1400,6 +1400,54 @@ void test_wire_set_state_then_commit(void) { TEST_ASSERT_EQUAL(1, g_live_int_setter_count); } +void test_wire_commit_reports_storage_error(void) { + // Point storage at a regular file so ensure_directory()/save_namespace() + // cannot create namespace files beneath it. The wire Commit response must + // report StorageError instead of claiming that the change was durable. + Provisioner::instance().end(); + register_custom_namespace(); + const std::string blocked_root = g_test_root + "/not_a_directory"; + FILE* blocker = fopen(blocked_root.c_str(), "wb"); + TEST_ASSERT_NOT_NULL(blocker); + fputs("block", blocker); + fclose(blocker); + Provisioner::instance().begin(blocked_root.c_str()); + + auto& p = Provisioner::instance(); + TEST_ASSERT_TRUE(p.field(CUSTOM_NS_ID, CUSTOM_INT, Value((int64_t)88))); + Bytes req = make_request((uint8_t)Op::Commit, 2, [](MsgPack::Packer& pk) { + MsgPack::object::nil_t n; + pk.serialize(n); + }); + Bytes resp = p.handle_message(req); + + MsgPack::Unpacker u; + u.feed(resp.data(), resp.size()); + TEST_ASSERT_TRUE(u.isArray()); + TEST_ASSERT_EQUAL_size_t(3, u.unpackArraySize()); + uint8_t op = 0; u.deserialize(op); + uint64_t seq = 0; u.deserialize(seq); + TEST_ASSERT_EQUAL((uint8_t)Op::Error, op); + TEST_ASSERT_EQUAL_UINT64(2, seq); + TEST_ASSERT_TRUE(u.isMap()); + const size_t n = u.unpackMapSize(); + bool found_storage_error = false; + for (size_t i = 0; i < n; ++i) { + int64_t key = 0; u.deserialize(key); + if (key == Key::ErrorCodeKey) { + int64_t code = 0; u.deserialize(code); + found_storage_error = code == (int64_t)ErrorCode::StorageError; + } + else if (u.isStr()) { + MsgPack::str_t ignored; u.deserialize(ignored); + } + else { + int64_t ignored = 0; u.deserialize(ignored); + } + } + TEST_ASSERT_TRUE(found_storage_error); +} + void test_wire_set_state_constraint_error(void) { fresh_provisioning(g_test_root); auto& p = Provisioner::instance(); @@ -1648,6 +1696,7 @@ int runUnityTests(void) { RUN_TEST(test_on_commit_callback_scoped_per_namespace); RUN_TEST(test_wire_get_info); RUN_TEST(test_wire_set_state_then_commit); + RUN_TEST(test_wire_commit_reports_storage_error); RUN_TEST(test_wire_set_state_constraint_error); RUN_TEST(test_wire_get_capabilities); RUN_TEST(test_wire_factory_reset);