diff --git a/CHANGELOG.md b/CHANGELOG.md index 1cf965e..33aed42 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,17 @@ project uses [semantic versioning][semver]. ### Fixed +- **A source on a network share survives its session dropping.** The retry + machinery reopened a dead handle and resumed from the last delivered chunk + already; the classifier just did not recognise a dropped SMB session as + transient, so the retry never fired and the file failed outright. The + network-mount codes now qualify — including `ERROR_UNEXP_NET_ERR`, which + Explorer reports as **0x8007003B** before abandoning the whole transfer, and + `ECONNRESET`, `ENETRESET`, `EPIPE` and `ESTALE` on a POSIX mount. Observed + offloading from a NAS over a VPN link: the path pings clean either side of + the drop, which is exactly why the retry is worth making. Recovery costs one + re-read of the chunk in flight. + - **A decoder ffmpeg lacks is probed once per job, not per clip.** Extracting thumbnails from BRAW with a stock ffmpeg fails identically for every clip; each one still paid four doomed process spawns. The first clip of a suffix diff --git a/ROADMAP.md b/ROADMAP.md index 277e04f..ae77690 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -33,6 +33,10 @@ not only the file hashes — so a rename or a moved file, which every file hash agrees is fine, is reported as the structure-hash mismatch it is. See [`docs/ascmhl.md`](docs/ascmhl.md#directory-hashes). +A source on a network mount is handled like marginal media, because it fails +like it: the dropped-session error codes are retried, the handle reopened and +the read resumed from the last delivered chunk. + `--paranoid` reads every source file a second time and compares, which is the only thing that catches a read returning wrong bytes without reporting an error. Retry works at the chunk that failed rather than restarting the file. Sidecars diff --git a/docs/data-safety.md b/docs/data-safety.md index 8eea8cd..6741dcd 100644 --- a/docs/data-safety.md +++ b/docs/data-safety.md @@ -202,7 +202,7 @@ known. - **Concurrent instances.** One app instance serialises its queue. Two instances pointed at the same destination are not coordinated. -## Marginal media +## Marginal media and dropped links Cards and readers fail intermittently long before they fail for good. A read that fails for a transient-looking reason is retried — three attempts by @@ -216,6 +216,20 @@ delay, so only errors with a plausible transient cause qualify: `EIO`, `EBUSY`, `ERROR_SHARING_VIOLATION` (usually antivirus, usually brief) and `ERROR_IO_DEVICE`. `ENOENT` and `ENOSPC` fail immediately. +A source on a network share qualifies for the same treatment, because it fails +the same way: an SMB session over a VPN or a WAN link dies mid-transfer and +takes the open handle with it, while the path is perfectly reachable either side +of the drop. So the network-mount codes are retried too — `ERROR_BAD_NETPATH`, +`ERROR_NETWORK_BUSY`, `ERROR_BAD_NET_RESP`, `ERROR_UNEXP_NET_ERR`, +`ERROR_NETNAME_DELETED`, `ERROR_REQ_NOT_ACCEP`, the two unreachables and +`ERROR_NO_SYSTEM_RESOURCES`, plus `ECONNRESET`, `ENETRESET`, `EPIPE` and +`ESTALE` on a POSIX mount. A read proves the path resolved a moment earlier, so +mid-copy these are a session that died rather than a name that was never right. + +`ERROR_UNEXP_NET_ERR` is the one Explorer reports as **0x8007003B**, "an +unexpected network error occurred", before abandoning the entire transfer. Here +it costs one re-read of the chunk in flight. + A failed *read* is retried at the chunk it failed on, not by restarting the file. A chunk is only hashed once it has arrived whole, so a read that failed produced no checksum state to unwind — recovering a bad sector near the end of a diff --git a/src/offloader/retry.py b/src/offloader/retry.py index c2edb03..707a269 100644 --- a/src/offloader/retry.py +++ b/src/offloader/retry.py @@ -5,6 +5,12 @@ antivirus holds a handle open for a beat. robocopy has retried this way for decades (`/R`, `/W`) and it is the main thing it does that this engine did not. +A network source fails the same way for different reasons. An SMB session over a +VPN or a WAN link dies mid-transfer and the handle with it, while the path stays +perfectly reachable either side of the drop — the case Explorer's copy engine +gives up on and robocopy rides out. Those codes are here for the same reason the +media ones are: the fault is in the link, not in the data. + The discrimination matters more than the retrying. Retrying a missing file, a permission denial or a full disk wastes time and hides the real problem, so only errors with a plausible transient cause are retried, and a file that needed one @@ -31,6 +37,16 @@ errno.ETIMEDOUT, errno.ENODEV, # device dropped off the bus and may come back errno.ENXIO, + # A network mount that dropped its session. Only ever consulted on POSIX: + # `is_transient` short-circuits on winerror, which Windows always sets. + errno.ECONNRESET, + errno.ECONNABORTED, + errno.ENETRESET, + errno.ENETUNREACH, + errno.EHOSTUNREACH, + errno.ENOTCONN, + errno.EPIPE, + errno.ESTALE, # the NFS classic: handle outlived the thing it named } #: Windows error codes worth a second attempt. Python surfaces these on @@ -40,11 +56,22 @@ 23, # ERROR_CRC — "Data error (cyclic redundancy check)" 32, # ERROR_SHARING_VIOLATION — usually antivirus, and usually brief 33, # ERROR_LOCK_VIOLATION + # The network-mount family. A read mid-copy proves the path resolved a + # moment ago, so these are a session that died under us rather than a name + # that was never right — the distinction that makes them worth retrying. + 53, # ERROR_BAD_NETPATH + 54, # ERROR_NETWORK_BUSY + 58, # ERROR_BAD_NET_RESP + 59, # ERROR_UNEXP_NET_ERR — what Explorer surfaces as 0x8007003B 64, # ERROR_NETNAME_DELETED — network destination blipped + 71, # ERROR_REQ_NOT_ACCEP — server at its connection limit 121, # ERROR_SEM_TIMEOUT 170, # ERROR_BUSY 1117, # ERROR_IO_DEVICE 1167, # ERROR_DEVICE_NOT_CONNECTED + 1231, # ERROR_NETWORK_UNREACHABLE + 1232, # ERROR_HOST_UNREACHABLE + 1450, # ERROR_NO_SYSTEM_RESOURCES — seen on sustained large SMB reads } class UnstableRead(OSError): diff --git a/tests/test_retry.py b/tests/test_retry.py index 849ad7e..e641c8e 100644 --- a/tests/test_retry.py +++ b/tests/test_retry.py @@ -54,6 +54,56 @@ def test_windows_error_codes(winerror: int, expected: bool): assert retry.is_transient(_os_error(errno.EIO, winerror=winerror)) is expected +@pytest.mark.parametrize("winerror", [53, 54, 58, 59, 64, 71, 1231, 1232, 1450]) +def test_dropped_network_session_is_retried(winerror: int): + """A source on an SMB share over a VPN loses its session mid-copy and the + handle with it. 59 (ERROR_UNEXP_NET_ERR) is the one Explorer reports as + 0x8007003B before abandoning the whole transfer; the engine reopens and + resumes from the last delivered chunk instead.""" + assert retry.is_transient(_os_error(errno.EIO, winerror=winerror)) + + +@pytest.mark.parametrize("code", [errno.ECONNRESET, errno.ENETRESET, + errno.EHOSTUNREACH, errno.ENOTCONN, + errno.EPIPE, errno.ESTALE]) +def test_posix_network_mount_errors_are_retried(code: int): + """The same drop seen through a POSIX mount, where there is no winerror to + short-circuit on.""" + assert retry.is_transient(_os_error(code)) + + +def test_a_windows_error_code_decides_over_the_mapped_errno(): + """Python maps a winerror onto whichever errno it thinks fits, and for a + dropped session that mapping can land on something in the permanent set. + The winerror is the specific fact and has to win, or the errno silently + vetoes a code that was deliberately added.""" + dropped = _os_error(errno.ENOENT, winerror=59) + assert retry.is_transient(dropped) + + +def test_a_permanent_windows_code_is_not_rescued_by_a_transient_errno(): + """The precedence has to cut both ways, or it is not precedence — it is + just a second chance for anything with the right errno.""" + denied = _os_error(errno.EIO, winerror=5) + assert not retry.is_transient(denied) + + +def test_an_unknown_windows_code_is_not_retried(): + """The default is to fail. A code nobody has reasoned about is not given + the benefit of the doubt, because the cost of guessing wrong is a delay + that hides the real fault.""" + assert not retry.is_transient(_os_error(errno.EIO, winerror=999999)) + + +def test_exhausted_wrapping_a_network_drop_is_not_retried_again(): + """A chunk-level loop raises Exhausted once it has spent its attempts. + Retrying it at a coarser level would repeat the same attempts against the + same dead session and re-read everything that already succeeded.""" + spent = retry.Exhausted("read failed at offset 0 after 3 attempts") + spent.winerror = 59 + assert not retry.is_transient(spent) + + def test_non_os_errors_are_never_retried(): assert not retry.is_transient(ValueError("nope")) assert not retry.is_transient(KeyboardInterrupt())