fix(filemover): log a blocked or failed mutation above Information - #853
Open
m4bard wants to merge 1 commit into
Open
fix(filemover): log a blocked or failed mutation above Information#853m4bard wants to merge 1 commit into
m4bard wants to merge 1 commit into
Conversation
LogMutation logged every outcome at Information, so a refusal to perform the requested action sat at the same level as a successful one. Blocked and Failed both mean the mutation did not happen. That matters most for the Unix cross-volume Move refusal, which is deliberate and says so in its reason: the source retirement cannot be generation-fenced without a library-side namespace claim. The refusal is not the bug. Its visibility is. The caller gets back a bare false, FileMutationResult is constructed and discarded with no consumer anywhere outside this class, and the operator is left watching a file that never arrives with nothing in the log that reads as a problem. None of the seven Blocked reasons in this class is routine chatter. Each is a refusal to do what was asked, so the level is wrong for all of them rather than only for the cross-volume one. The test drives the real path through the existing ForceCrossVolumeForTest hook and asserts the level rather than the text, using the CapturingLogger shape already used by the NZBGet and Usenet adapter tests. CreateMover gains an optional logger; it defaulted to NullLogger, which is why no existing test could see this.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
LogMutationlogs every outcome atInformation, so a refusal to perform a file mutation sits at the same level as a successful one.BlockedandFailedboth mean the action did not happen.This changes the level for those two and nothing else. Full write-up in #852.
Changes
Fixed
FileMover.LogMutationselectsWarningforBlockedandFailed,Informationotherwise.Why this and not more
The case that surfaced it is the Unix cross-volume
Moverefusal, which is deliberate and states its reason: the source retirement cannot be generation-fenced without a library-side namespace claim. That refusal is not touched here and I am not proposing it should be. What this changes is that an operator watching at the default level can now tell the difference between a move that was refused and a move that succeeded.It applies to all outcomes rather than special-casing the cross-volume one because none of the seven
Blockedsites in this class is routine: a non-publishable action, an empty operation ID, unavailable durable markerless state, linked aliases of the same file. Every one is a refusal to do what was asked.Worth stating plainly: this does not make cross-volume moves work. It makes their refusal audible. Whether to implement the namespace claim described in that reason is a separate and much larger question.
Testing
FileMoverMarkerlessMoveTestsgains a case that drives the real refusal through the existingForceCrossVolumeForTesthook and asserts the level rather than the message text.CreateMovergains an optionalILogger<FileMover>; it previously passedNullLoggerunconditionally, which is why no existing test in that file could observe a level. There is already a test asserting this move is rejected, so the gap was never whether it refuses, only how loudly.Verified as a real guard: with the level flattened back to
Informationthe new test fails withExpected: Warning, Actual: Information, and passes with the change.Full suite: 3,030 passed, 0 failed, 125 skipped, against a 3,029 baseline on
03958c15.Reproduced end to end against
ghcr.io/listenarrs/listenarr:canarywith a public check that runs the same import twice, once with the source and root folder on one mount and once across two. The same-mount case is the control and completes; the cross-mount case produces no destination and leaves the source untouched. The check is in the test-data repo linked from the issue, and it reports "nothing happened at all" as its own verdict rather than as an ordinary failure, since a stall and a failure are different things to operate.