Skip to content
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,16 @@ 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 cancel during a second destination's reread left the file reading as
Verified.** Only the destinations already promoted were recorded, and a
file's status is its weakest recorded copy, so the copy that was never made
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ ruff check src tests
pytest --cov=offloader --cov-report=term-missing
```

485 tests at 86% line coverage. They cover formatting against the reference's
509 tests at 86% line coverage. They cover formatting against the reference's
exact strings, checksum vectors and streaming equivalence, copy/verify
behaviour including simulated destination corruption, pause/resume/cancel
concurrency, retry discrimination, BRAW container parsing, ffprobe parsing,
Expand Down
4 changes: 4 additions & 0 deletions ROADMAP.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 15 additions & 1 deletion docs/data-safety.md
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,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
Expand All @@ -218,6 +218,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
Expand Down
27 changes: 27 additions & 0 deletions src/offloader/retry.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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):
Expand Down
59 changes: 56 additions & 3 deletions tests/test_retry.py
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -453,14 +503,17 @@ def test_a_bad_sector_is_recovered_without_re_reading_the_file(
assert log.count(0) == 1, f"the file was restarted: {log}"


@pytest.mark.parametrize("reopen_winerror", [1117, 59, 64])
def test_a_reopen_that_fails_spends_an_attempt_not_the_offload(
tmp_path: Path, monkeypatch
tmp_path: Path, monkeypatch, reopen_winerror: int
):
"""REGRESSION. Recovery used to run in `before_retry`, which `retry.call`
invokes outside the clause that catches OSError. A reopen that failed
therefore escaped with attempts still unspent, and the handler wrapped it in
`Exhausted`, closing the whole-file retry as well. A reader that takes a
moment to come back costs one attempt of the chunk's budget."""
moment to come back costs one attempt of the chunk's budget. The 59 and 64
cases are a network share whose session is still down when the reopen is
tried: classified transient, so retried inside the chunk's budget."""
monkeypatch.setattr(engine, "CHUNK_SIZE", 4096)
card, payload = _chunked_card(tmp_path, 3)
log: list[int] = []
Expand All @@ -479,7 +532,7 @@ def flaky_open(path, mode="r", *args, **kwargs):
# this is. The handle does not come back on the first try.
if failures["n"] and not reopens["n"]:
reopens["n"] += 1
raise _os_error(errno.EIO, winerror=1117)
raise _os_error(errno.EIO, winerror=reopen_winerror)
return _BadSector(real_open(path, mode, *args, **kwargs),
log, failures, 4096, 1)
return real_open(path, mode, *args, **kwargs)
Expand Down