diff --git a/docs/api/file-sink-reopen-append.md b/docs/api/file-sink-reopen-append.md index 6a6a292..74ca3f8 100644 --- a/docs/api/file-sink-reopen-append.md +++ b/docs/api/file-sink-reopen-append.md @@ -2,7 +2,7 @@ name: file-sink-reopen-append group: api category: sink -update-time: 20260613 +update-time: 20260717 description: Reopen a FileSink in append mode. key-word: - file @@ -37,6 +37,7 @@ Detailed rules explaining key parameters and behaviors - Reopen behavior is fixed to append mode. - The stored append policy is updated to `true` as part of the reopen path. - On failure, the sink remains unavailable and `open_failures` is incremented. +- After a rotation failure, this is the explicit recovery path once the underlying remove, rename, or open condition has been resolved. ### How to Use diff --git a/docs/api/file-sink-rotation-failures.md b/docs/api/file-sink-rotation-failures.md index f4fb4f7..fb803d4 100644 --- a/docs/api/file-sink-rotation-failures.md +++ b/docs/api/file-sink-rotation-failures.md @@ -2,7 +2,7 @@ name: file-sink-rotation-failures group: api category: sink -update-time: 20260707 +update-time: 20260717 description: Read the number of rotation failures recorded by a FileSink. key-word: - file @@ -36,6 +36,7 @@ Detailed rules explaining key parameters and behaviors - The sink reports its recorded rotation-failure count directly. - The counter increases when a rotation attempt cannot complete its critical file steps, such as removing the active file with zero backups, renaming a live file into `.1`, shifting an existing backup to a higher slot, or reopening the fresh active file afterward. - Missing optional backup slots do not count by themselves; only steps that fail while their source path still exists are treated as rotation failures. +- When a critical rotation step fails, the triggering record is not written and the sink remains unavailable until an explicit reopen succeeds. - The counter is cumulative until reset. - This helper is observation-only and does not mutate sink state. @@ -70,6 +71,8 @@ e.g.: - This counter alone does not say whether the sink is currently available. +- After a recorded rotation failure, use `reopen_append()` only after the underlying filesystem condition has been resolved. + ### Notes 1. Use this helper when diagnosing incomplete direct file rotations. diff --git a/src/BitLogger_wbtest.mbt b/src/BitLogger_wbtest.mbt index 7891034..20fbb1b 100644 --- a/src/BitLogger_wbtest.mbt +++ b/src/BitLogger_wbtest.mbt @@ -105,57 +105,3 @@ test "file sink can rotate on native backend" { inspect(sink.rotation_failures(), content="0") } } - -///| -#cfg(platform="windows") -test "file sink rotation failure counts partial backup-chain failure on native backend" { - let path = "logs/bitlogger-rotate-partial-failure.log" - ignore(@utils.remove_file_internal(path)) - ignore(@utils.remove_file_internal(path + ".1")) - ignore(@utils.remove_file_internal(path + ".2")) - - let seed = file_sink( - path, - auto_flush=true, - rotation=Some(file_rotation(20, max_backups=2)), - formatter=fn(rec) { rec.message }, - ) - if seed.is_available() { - seed.write(record(Level::Info, "1234567890")) - seed.write(record(Level::Info, "abcdefghij")) - inspect(seed.rotation_failures(), content="0") - inspect(seed.close(), content="true") - - let locked = @utils.open_file_handle_internal(path + ".2", true) - inspect(locked is Some(_), content="true") - - let sink = file_sink( - path, - auto_flush=true, - rotation=Some(file_rotation(20, max_backups=2)), - formatter=fn(rec) { rec.message }, - ) - inspect(sink.is_available(), content="true") - sink.write(record(Level::Info, "klmnopqrst")) - - inspect(sink.rotation_failures(), content="1") - inspect(sink.state().rotation_failures, content="1") - inspect(sink.write_failures(), content="1") - inspect(sink.is_available(), content="false") - - match locked { - None => () - Some(handle) => ignore(@utils.close_file_handle_internal(handle)) - } - - inspect(sink.reopen_append(), content="true") - inspect(sink.is_available(), content="true") - inspect(sink.close(), content="true") - - ignore(@utils.remove_file_internal(path)) - ignore(@utils.remove_file_internal(path + ".1")) - ignore(@utils.remove_file_internal(path + ".2")) - } else { - inspect(native_files_supported(), content="false") - } -} diff --git a/src/file_runtime/file_sink.mbt b/src/file_runtime/file_sink.mbt index 25ef1f8..e9aea64 100644 --- a/src/file_runtime/file_sink.mbt +++ b/src/file_runtime/file_sink.mbt @@ -16,6 +16,24 @@ type Record = @core.Record ///| type RecordFormatter = @formatting.RecordFormatter +///| +priv struct RotationFileOps { + file_exists : (String) -> Bool + remove_file : (String) -> Bool + rename_file : (String, String) -> Bool +} + +///| +fn default_rotation_file_ops() -> RotationFileOps { + { + file_exists: fn(path) { @utils.file_exists_internal(path) }, + remove_file: fn(path) { @utils.remove_file_internal(path) }, + rename_file: fn(from_path, to_path) { + @utils.rename_file_internal(from_path, to_path) + }, + } +} + ///| pub struct FileSink { priv path : String @@ -31,6 +49,7 @@ pub struct FileSink { priv write_failures : Ref[Int] priv flush_failures : Ref[Int] priv rotation_failures : Ref[Int] + priv rotation_file_ops : Ref[RotationFileOps] } ///| @@ -61,6 +80,7 @@ pub fn file_sink( write_failures: Ref(0), flush_failures: Ref(0), rotation_failures: Ref(0), + rotation_file_ops: Ref(default_rotation_file_ops()), } } @@ -291,6 +311,28 @@ fn rotated_file_path(path : String, index : Int) -> String { "\{path}.\{index}" } +///| +fn remove_file_if_present(ops : RotationFileOps, path : String) -> Bool { + if !(ops.file_exists)(path) { + true + } else { + (ops.remove_file)(path) || !(ops.file_exists)(path) + } +} + +///| +fn rename_file_if_present( + ops : RotationFileOps, + from_path : String, + to_path : String, +) -> Bool { + if !(ops.file_exists)(from_path) { + true + } else { + (ops.rename_file)(from_path, to_path) || !(ops.file_exists)(from_path) + } +} + ///| fn rotate_file_sink_internal(sink : FileSink, rotation : FileRotation) -> Bool { let closed = match sink.handle.val { @@ -304,23 +346,27 @@ fn rotate_file_sink_internal(sink : FileSink, rotation : FileRotation) -> Bool { if !closed { return false } + let ops = sink.rotation_file_ops.val if rotation.max_backups > 0 { - ignore( - @utils.remove_file_internal( + if !remove_file_if_present( + ops, rotated_file_path(sink.path, rotation.max_backups), - ), - ) + ) { + return false + } for index = rotation.max_backups - 1; index >= 1; { let from_path = rotated_file_path(sink.path, index) let to_path = rotated_file_path(sink.path, index + 1) - ignore(@utils.rename_file_internal(from_path, to_path)) + if !rename_file_if_present(ops, from_path, to_path) { + return false + } continue index - 1 } - ignore( - @utils.rename_file_internal(sink.path, rotated_file_path(sink.path, 1)), - ) - } else { - ignore(@utils.remove_file_internal(sink.path)) + if !rename_file_if_present(ops, sink.path, rotated_file_path(sink.path, 1)) { + return false + } + } else if !remove_file_if_present(ops, sink.path) { + return false } sink.handle.val = @utils.open_file_handle_internal(sink.path, false) sink.handle.val is Some(_) diff --git a/src/file_runtime/file_sink_rotation_wbtest.mbt b/src/file_runtime/file_sink_rotation_wbtest.mbt new file mode 100644 index 0000000..46971bb --- /dev/null +++ b/src/file_runtime/file_sink_rotation_wbtest.mbt @@ -0,0 +1,111 @@ +///| +test "file sink counts an injected partial backup-chain failure" { + let test_path = "logs/file-runtime-rotate-partial-failure.log" + ignore(@utils.remove_file_internal(test_path)) + ignore(@utils.remove_file_internal(test_path + ".1")) + ignore(@utils.remove_file_internal(test_path + ".2")) + + let rotation = @file_model.file_rotation(20, max_backups=2) + let seed = file_sink(test_path, auto_flush=true, rotation=Some(rotation), formatter=fn( + rec, + ) { + rec.message + }) + if seed.is_available() { + seed.write(@core.Record::new(@core.Level::Info, "1234567890")) + seed.write(@core.Record::new(@core.Level::Info, "abcdefghij")) + inspect(seed.rotation_failures(), content="0") + inspect(seed.close(), content="true") + + let sink = file_sink(test_path, auto_flush=true, rotation=Some(rotation), formatter=fn( + rec, + ) { + rec.message + }) + sink.rotation_file_ops.val = { + file_exists: fn(candidate) { @utils.file_exists_internal(candidate) }, + remove_file: fn(candidate) { @utils.remove_file_internal(candidate) }, + rename_file: fn(from_path, to_path) { + if from_path == test_path + ".1" && to_path == test_path + ".2" { + false + } else { + @utils.rename_file_internal(from_path, to_path) + } + }, + } + sink.write(@core.Record::new(@core.Level::Info, "klmnopqrst")) + + inspect(sink.rotation_failures(), content="1") + inspect(sink.state().rotation_failures, content="1") + inspect(sink.write_failures(), content="1") + inspect(sink.is_available(), content="false") + inspect(sink.reopen_append(), content="true") + inspect(sink.is_available(), content="true") + inspect(sink.close(), content="true") + + ignore(@utils.remove_file_internal(test_path)) + ignore(@utils.remove_file_internal(test_path + ".1")) + ignore(@utils.remove_file_internal(test_path + ".2")) + } else { + inspect(native_files_supported(), content="false") + } +} + +///| +test "file sink counts an injected oldest-backup removal failure" { + let test_path = "logs/file-runtime-rotate-removal-failure.log" + ignore(@utils.remove_file_internal(test_path)) + ignore(@utils.remove_file_internal(test_path + ".1")) + ignore(@utils.remove_file_internal(test_path + ".2")) + + let rotation = @file_model.file_rotation(20, max_backups=2) + let seed = file_sink(test_path, auto_flush=true, rotation=Some(rotation), formatter=fn( + rec, + ) { + rec.message + }) + if seed.is_available() { + seed.write(@core.Record::new(@core.Level::Info, "1234567890")) + seed.write(@core.Record::new(@core.Level::Info, "abcdefghij")) + inspect(seed.close(), content="true") + + let oldest = @utils.open_file_handle_internal(test_path + ".2", true) + inspect(oldest is Some(_), content="true") + match oldest { + None => () + Some(handle) => + inspect(@utils.close_file_handle_internal(handle), content="true") + } + + let sink = file_sink(test_path, auto_flush=true, rotation=Some(rotation), formatter=fn( + rec, + ) { + rec.message + }) + sink.rotation_file_ops.val = { + file_exists: fn(candidate) { @utils.file_exists_internal(candidate) }, + remove_file: fn(candidate) { + if candidate == test_path + ".2" { + false + } else { + @utils.remove_file_internal(candidate) + } + }, + rename_file: fn(from_path, to_path) { + @utils.rename_file_internal(from_path, to_path) + }, + } + sink.write(@core.Record::new(@core.Level::Info, "klmnopqrst")) + + inspect(sink.rotation_failures(), content="1") + inspect(sink.write_failures(), content="1") + inspect(sink.is_available(), content="false") + inspect(sink.close(), content="false") + + ignore(@utils.remove_file_internal(test_path)) + ignore(@utils.remove_file_internal(test_path + ".1")) + ignore(@utils.remove_file_internal(test_path + ".2")) + } else { + inspect(native_files_supported(), content="false") + } +} diff --git a/src/utils/file_backend_stub.mbt b/src/utils/file_backend_stub.mbt index 0733a57..fb6ce67 100644 --- a/src/utils/file_backend_stub.mbt +++ b/src/utils/file_backend_stub.mbt @@ -12,6 +12,12 @@ pub fn open_file_handle_internal(path : String, append : Bool) -> FileHandle? { None } +///| +pub fn file_exists_internal(path : String) -> Bool { + ignore(path) + false +} + ///| pub fn write_file_handle_internal( handle : FileHandle, diff --git a/src/utils/pkg.generated.mbti b/src/utils/pkg.generated.mbti index 319eeb9..65754f0 100644 --- a/src/utils/pkg.generated.mbti +++ b/src/utils/pkg.generated.mbti @@ -25,6 +25,8 @@ pub fn default_style_tag_registry() -> @formatting.StyleTagRegistry pub fn field_equals(String, String) -> (@core.Record) -> Bool +pub fn file_exists_internal(String) -> Bool + pub fn file_size_i64_internal(FileHandle) -> Int64 pub fn flush_file_handle_internal(FileHandle) -> Bool