fix: report failed transfers instead of ending in success

Upload and download failures were printed and then forgotten: a batch
that lost files still exited 0, and a file the client could not write
was listed as if it had arrived. Nothing downstream could tell.

- Return a verdict from every transfer worker and raise once at the
  end, so a run that lost a file exits 2.
- Raise the error when a downloaded file cannot be written locally,
  instead of printing it and reporting the path as a success.
- Write a download beside its destination and move it into place once
  complete, and refuse a destination that cannot be written, so a
  failed transfer no longer leaves a truncated file behind.
- Carry on through the remaining sub-folders when a recursive download
  loses a file or cannot create a folder locally.
- Name the file and give the reason in every failure message.
- Count a failure the API layer did not raise, such as a file that
  disappeared between the directory walk and its turn to be sent.
- Extract the directory walk from `Uploader.upload` to keep it within
  the complexity limit.
- Cover all of the above in `tests/test_transfer.py`.

CHANGELOG.md

@@ -2,6 +2,25 @@
 
 All notable changes to this project will be documented in this file.
 
+## [Unreleased]
+
+### Fixed
+- Reported a failed upload to the caller. A file the server refused was printed and then forgotten, so a batch that lost files still ended in success and scripts could not tell.
+- Reported a failed single-file download to the caller, which was counted internally but never raised, so only recursive downloads ever ended in failure.
+- Raised the error when a downloaded file could not be written locally. A permission error was printed and the path was then listed as if the file had arrived.
+- Wrote a download to a temporary file beside its destination and moved it into place once the whole body had arrived. A transfer that failed part way used to leave a truncated file under the real name, and a destination that could not be opened was then deleted even though nothing had been written to it. A destination the client may not write is now refused before anything is fetched.
+- Named the file in the message when an upload failed, and counted a failure the API layer did not raise, such as a file that disappeared between the directory walk and its turn to be sent.
+- Carried on through the remaining sub-folders when a recursive download lost a file or could not be created locally, instead of abandoning the rest of the tree at the first failure. Every failure is now printed against its own file and reported once at the end.
+- Said why a download failed, rather than printing the path alone.
+- Serialised the token refresh across processes. Concurrent `mdrs` invocations shared one refresh token and each sent it, so a provider that rotates refresh tokens accepted the first and refused the rest.
+- Wrote the login cache through a temporary file so a reader can no longer catch it mid-truncation and discard the session.
+- Joined the base URL and the API's relative path correctly in `ls --json` output, which produced a doubled separator when the configured URL ended with one.
+
+### Changed
+- Accepted bare hostnames such as `localhost` in `config create`/`config update`, and stored the URL without its trailing slash, matching the Rust client so both can share `config.ini`.
+- Bounded the token refresh request with its own timeout, so a provider that stops answering cannot hold the cross-process lock indefinitely.
+- **Breaking for embedders:** `CacheInterface` now requires `lock()` and `reload()`. A cache passed to `MdrsClient.from_remote(..., cache=...)` must provide both; `InMemoryCache` implements them as no-ops.
+
 ## [1.3.18] - 2026-07-02
 
 ### Added
@@ -37,36 +56,41 @@ All notable changes to this project will be documented in this file.
 
 ## [1.3.15] - 2026-05-01
 
-### Changed
-- Bumped package version to 1.3.15.
+### Fixed
+- Apply NFC normalization to filenames and folder names sent to the server.
 
 ## [1.3.14] - 2026-04-17
 
 ### Changed
 - Simplified `config list` command (removed `-l`/`--long` option, always display URL).
+- Renamed `--quick` option to `--quiet` for `ls` subcommand.
 
 ### Added
 - Added subcommand aliases for config commands (e.g. `ls` alias for list, `rm` alias for delete).
+- Added `version` command.
 
 ## [1.3.13] - 2025-07-02
 
-### Changed
-- Bumped package version to 1.3.13.
+### Fixed
+- Fixed pagination logic for the `file.list` API.
 
 ## [1.3.12] - 2025-05-20
 
-### Changed
-- Bumped package version to 1.3.12.
+### Fixed
+- Fixed bug where file downloading was skipped incorrectly when `-s`/`--skip-if-file-exists` option was present.
 
 ## [1.3.11] - 2025-01-21
 
-### Changed
-- Bumped package version to 1.3.11.
+### Fixed
+- Follow-up fixes for User API specification changes.
 
 ## [1.3.10] - 2024-12-23
 
+### Added
+- Delete broken files and show a summary when a file download fails.
+
 ### Changed
-- Bumped package version to 1.3.10.
+- Updated dependency libraries.
 
 ## [1.3.9] - 2024-10-23
 
@@ -75,8 +99,11 @@ All notable changes to this project will be documented in this file.
 
 ## [1.3.8] - 2024-09-18
 
-### Changed
-- Bumped package version to 1.3.8.
+### Added
+- Implemented `-s`/`--skip-if-file-exists` option for `download` command.
+
+### Fixed
+- Added exception handling for unexpected responses from the server.
 
 ## [1.3.7] - 2024-07-22
 
@@ -85,23 +112,29 @@ All notable changes to this project will be documented in this file.
 
 ## [1.3.6] - 2024-07-08
 
-### Changed
-- Bumped package version to 1.3.6.
+### Added
+- Support cancelling recursive downloads if downloading some files fails.
 
 ## [1.3.5] - 2024-07-08
 
-### Changed
-- Bumped package version to 1.3.5.
+### Added
+- Added authorization token validation checks for file download operations.
+
+### Removed
+- Removed unnecessary debug code.
 
 ## [1.3.4] - 2024-07-04
 
 ### Added
 - Added some aliases for config sub command.
 
+### Fixed
+- Fixed bug when uploading large files.
+
 ## [1.3.3] - 2024-02-13
 
-### Changed
-- Bumped package version to 1.3.3.
+### Added
+- Implemented `-s`/`--skip-if-file-exists` option for `upload` command.
 
 ## [1.3.2] - 2024-02-09
 

README.md

@@ -2,10 +2,42 @@
 
 The mdrs-client-python is python library and a command-line client for up- and downloading files to and from MDRS based repository.
 
-## Installing
+## Installation (Production)
+
+It is recommended to use a virtual environment (`venv`) to keep your Python environment isolated.
+
+### Option 1: Using standard `venv` + `pip`
 
 ```shell
-poetry install
+python3 -m venv .venv
+source .venv/bin/activate  # On Windows: .venv\Scripts\activate
+pip install .
+```
+
+### Option 2: Using `uv`
+
+```shell
+uv venv
+source .venv/bin/activate  # On Windows: .venv\Scripts\activate
+uv pip install .
+```
+
+## Development Setup
+
+To set up a local development environment with development tools (testing, formatting, linting):
+
+### Option 1: Using standard `venv` + `pip`
+
+```shell
+python3 -m venv .venv
+source .venv/bin/activate  # On Windows: .venv\Scripts\activate
+pip install -e ".[dev]"
+```
+
+### Option 2: Using `uv` (Recommended for developers)
+
+```shell
+uv sync
 ```
 
 ## CLI Usage

mdrsclient/api/base.py

@@ -5,7 +5,13 @@ from pydantic import TypeAdapter
 from requests import Response
 
 from mdrsclient.connection import MDRSConnection
-from mdrsclient.exceptions import BadRequestException, ForbiddenException, UnauthorizedException, UnexpectedException
+from mdrsclient.exceptions import (
+    BadRequestException,
+    ForbiddenException,
+    TokenExpiredException,
+    UnauthorizedException,
+    UnexpectedException,
+)
 from mdrsclient.models.error import DRFStandardizedErrors
 
 
@@ -23,6 +29,11 @@ class BaseApi(ABC):
             if response.status_code == requests.codes.bad_request:
                 raise BadRequestException(errors.errors[0].detail)
             elif response.status_code == requests.codes.unauthorized:
+                # A request can wait in the server's queue for longer than the access
+                # token it was sent with lives, so an expired token here does not mean
+                # the session is over - it means this one request arrived too late.
+                if any(e.code == "token_not_valid" for e in errors.errors):
+                    raise TokenExpiredException("Access token expired before the request was served.")
                 raise UnauthorizedException("Login required.")
             elif response.status_code == requests.codes.forbidden:
                 raise ForbiddenException("You do not have enough permissions. Access is denied.")

mdrsclient/api/doi.py

@@ -4,7 +4,7 @@ from pydantic import TypeAdapter
 from pydantic.dataclasses import dataclass
 
 from mdrsclient.api.base import BaseApi
-from mdrsclient.api.utils import token_check
+from mdrsclient.api.utils import retry_on_expired_token, token_check
 from mdrsclient.models.doi import Doi
 
 
@@ -23,6 +23,7 @@ class DoiRetrieveResponse:
 class DoiApi(BaseApi):
     ENTRYPOINT: Final[str] = "v3/doi/"
 
+    @retry_on_expired_token
     def retrieve(self, doi_id: str) -> Doi:
         """Retrieve the folder associated with a DOI suffix ID (GET v3/doi/{id}/)."""
         url = self.ENTRYPOINT + doi_id + "/"

mdrsclient/api/files.py

@@ -1,5 +1,6 @@
 import mimetypes
 import os
+import threading
 from typing import Any, Final
 from unicodedata import normalize
 
@@ -8,8 +9,8 @@ from pydantic.dataclasses import dataclass
 from requests_toolbelt.multipart.encoder import MultipartEncoder
 
 from mdrsclient.api.base import BaseApi
-from mdrsclient.api.utils import token_check
-from mdrsclient.exceptions import UnexpectedException
+from mdrsclient.api.utils import retry_on_expired_token, token_check
+from mdrsclient.exceptions import MDRSException, UnexpectedException
 from mdrsclient.models import File
 
 
@@ -30,6 +31,7 @@ class FilesApi(BaseApi):
     ENTRYPOINT: Final[str] = "v3/files/"
     FALLBACK_MIMETYPE: Final[str] = "application/octet-stream"
 
+    @retry_on_expired_token
     def list(self, folder_id: str, page_num: int) -> FilesApiListResponse:
         url = self.ENTRYPOINT
         token_check(self.connection)
@@ -38,6 +40,7 @@ class FilesApi(BaseApi):
         self._raise_response_error(response)
         return TypeAdapter(FilesApiListResponse).validate_python(response.json())
 
+    @retry_on_expired_token
     def retrieve(self, id: str) -> File:
         # print(self.__class__.__name__ + "::" + sys._getframe().f_code.co_name)
         url = self.ENTRYPOINT + id + "/"
@@ -46,6 +49,7 @@ class FilesApi(BaseApi):
         self._raise_response_error(response)
         return TypeAdapter(File).validate_python(response.json())
 
+    @retry_on_expired_token
     def create(self, folder_id: str, path: str) -> str:
         # print(self.__class__.__name__ + "::" + sys._getframe().f_code.co_name)
         url = self.ENTRYPOINT
@@ -66,10 +70,14 @@ class FilesApi(BaseApi):
             raise UnexpectedException(f"Could not open `{path}` file.")
         except MemoryError:
             raise UnexpectedException("Out of memory.")
+        except MDRSException:
+            # Already says what went wrong, and the caller may want to act on the kind.
+            raise
         except Exception as e:
             raise UnexpectedException("Unspecified error.") from e
         return ret.id
 
+    @retry_on_expired_token
     def update(self, file: File, path: str | None) -> bool:
         # print(self.__class__.__name__ + "::" + sys._getframe().f_code.co_name)
         url = self.ENTRYPOINT + file.id + "/"
@@ -96,6 +104,7 @@ class FilesApi(BaseApi):
         self._raise_response_error(response)
         return True
 
+    @retry_on_expired_token
     def destroy(self, file: File) -> bool:
         # print(self.__class__.__name__ + "::" + sys._getframe().f_code.co_name)
         url = self.ENTRYPOINT + file.id + "/"
@@ -104,6 +113,7 @@ class FilesApi(BaseApi):
         self._raise_response_error(response)
         return True
 
+    @retry_on_expired_token
     def move(self, file: File, folder_id: str, name: str) -> bool:
         # print(self.__class__.__name__ + "::" + sys._getframe().f_code.co_name)
         url = self.ENTRYPOINT + file.id + "/move/"
@@ -113,6 +123,7 @@ class FilesApi(BaseApi):
         self._raise_response_error(response)
         return True
 
+    @retry_on_expired_token
     def copy(self, file: File, folder_id: str, name: str) -> bool:
         # print(self.__class__.__name__ + "::" + sys._getframe().f_code.co_name)
         url = self.ENTRYPOINT + file.id + "/copy/"
@@ -122,6 +133,7 @@ class FilesApi(BaseApi):
         self._raise_response_error(response)
         return True
 
+    @retry_on_expired_token
     def metadata(self, file: File) -> dict[str, Any]:
         # print(self.__class__.__name__ + "::" + sys._getframe().f_code.co_name)
         url = self.ENTRYPOINT + file.id + "/metadata/"
@@ -130,22 +142,58 @@ class FilesApi(BaseApi):
         self._raise_response_error(response)
         return response.json()
 
+    @retry_on_expired_token
     def download(self, file: File, path: str) -> bool:
         # print(self.__class__.__name__ + "::" + sys._getframe().f_code.co_name)
         url = file.download_url
         token_check(self.connection)
+        # Refused before anything is fetched. The finished file is moved into place, and a
+        # rename would replace a destination whose mode says it is protected.
+        if os.path.exists(path):
+            try:
+                with open(path, "r+b"):
+                    pass
+            except OSError as e:
+                raise UnexpectedException(f"Cannot write `{path}`: {e}")
         response = self.connection.get(url, stream=True)
         self._raise_response_error(response)
+        # Written beside the destination and moved in once the whole body has arrived, so
+        # a transfer that fails part way leaves whatever was already there untouched and
+        # never leaves a truncated file under the real name.
+        fd, tmp_path = self._open_partial(path)
         try:
-            with open(path, "wb") as f:
+            with os.fdopen(fd, "wb") as f:
                 for chunk in response.iter_content(chunk_size=4096):
                     if chunk:
                         f.write(chunk)
-                        f.flush()
-        except PermissionError:
-            print(f"Cannot create file `{path}`: Permission denied.")
+            os.replace(tmp_path, path)
+        except BaseException:
+            # Only the scratch file goes: anything at the destination was not written here.
+            if os.path.exists(tmp_path):
+                os.unlink(tmp_path)
+            raise
         return True
 
+    @staticmethod
+    def _open_partial(path: str) -> tuple[int, str]:
+        """
+        Create a scratch file beside `path` and return it open for writing.
+
+        Beside it, so moving the finished download into place is a rename within one
+        directory. `0o666` rather than a private mode because the umask is what decided
+        the permissions of a downloaded file before, and still should.
+        """
+        base = f"{path}.{os.getpid()}-{threading.get_ident()}"
+        for attempt in range(100):
+            tmp_path = f"{base}-{attempt}.mdrspart"
+            try:
+                return os.open(tmp_path, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o666), tmp_path
+            except FileExistsError:
+                continue
+            except OSError as e:
+                raise UnexpectedException(f"Cannot write `{path}`: {e}")
+        raise UnexpectedException(f"Could not create a temporary file beside `{path}`.")
+
     def _get_mime_type(self, path: str) -> str:
         mt = mimetypes.guess_type(path)
         if mt:

mdrsclient/api/folders.py

@@ -5,7 +5,7 @@ from pydantic import TypeAdapter
 from pydantic.dataclasses import dataclass
 
 from mdrsclient.api.base import BaseApi
-from mdrsclient.api.utils import token_check
+from mdrsclient.api.utils import retry_on_expired_token, token_check
 from mdrsclient.exceptions import UnauthorizedException
 from mdrsclient.models import Folder, FolderSimple
 
@@ -18,6 +18,7 @@ class FoldersApiCreateResponse:
 class FoldersApi(BaseApi):
     ENTRYPOINT: Final[str] = "v3/folders/"
 
+    @retry_on_expired_token
     def list(self, laboratory_id: int, path: str) -> list[FolderSimple]:
         # print(self.__class__.__name__ + "::" + sys._getframe().f_code.co_name)
         url = self.ENTRYPOINT
@@ -30,6 +31,7 @@ class FoldersApi(BaseApi):
             ret.append(TypeAdapter(FolderSimple).validate_python(data))
         return ret
 
+    @retry_on_expired_token
     def retrieve(self, id: str) -> Folder:
         # print(self.__class__.__name__ + "::" + sys._getframe().f_code.co_name)
         url = self.ENTRYPOINT + id + "/"
@@ -39,6 +41,7 @@ class FoldersApi(BaseApi):
         ret = TypeAdapter(Folder).validate_python(response.json())
         return ret
 
+    @retry_on_expired_token
     def create(self, name: str, parent_id: str) -> str:
         # print(self.__class__.__name__ + "::" + sys._getframe().f_code.co_name)
         url = self.ENTRYPOINT
@@ -49,6 +52,7 @@ class FoldersApi(BaseApi):
         ret = TypeAdapter(FoldersApiCreateResponse).validate_python(response.json())
         return ret.id
 
+    @retry_on_expired_token
     def update(self, folder: FolderSimple) -> bool:
         # print(self.__class__.__name__ + "::" + sys._getframe().f_code.co_name)
         url = self.ENTRYPOINT + folder.id + "/"
@@ -61,6 +65,7 @@ class FoldersApi(BaseApi):
         self._raise_response_error(response)
         return True
 
+    @retry_on_expired_token
     def destroy(self, id: str, recursive: bool) -> bool:
         # print(self.__class__.__name__ + "::" + sys._getframe().f_code.co_name)
         url = self.ENTRYPOINT + id + "/"
@@ -70,6 +75,7 @@ class FoldersApi(BaseApi):
         self._raise_response_error(response)
         return True
 
+    @retry_on_expired_token
     def auth(self, id: str, password: str) -> bool:
         # print(self.__class__.__name__ + "::" + sys._getframe().f_code.co_name)
         url = self.ENTRYPOINT + id + "/auth/"
@@ -81,6 +87,7 @@ class FoldersApi(BaseApi):
         self._raise_response_error(response)
         return True
 
+    @retry_on_expired_token
     def acl(self, id: str, access_level: int, recursive: bool, password: str | None) -> bool:
         # print(self.__class__.__name__ + "::" + sys._getframe().f_code.co_name)
         url = self.ENTRYPOINT + id + "/acl/"
@@ -94,6 +101,7 @@ class FoldersApi(BaseApi):
         self._raise_response_error(response)
         return True
 
+    @retry_on_expired_token
     def move(self, folder: FolderSimple, folder_id: str, name: str) -> bool:
         # print(self.__class__.__name__ + "::" + sys._getframe().f_code.co_name)
         url = self.ENTRYPOINT + folder.id + "/move/"
@@ -103,6 +111,7 @@ class FoldersApi(BaseApi):
         self._raise_response_error(response)
         return True
 
+    @retry_on_expired_token
     def copy(self, folder: FolderSimple, folder_id: str, name: str) -> bool:
         # print(self.__class__.__name__ + "::" + sys._getframe().f_code.co_name)
         url = self.ENTRYPOINT + folder.id + "/copy/"
@@ -112,6 +121,7 @@ class FoldersApi(BaseApi):
         self._raise_response_error(response)
         return True
 
+    @retry_on_expired_token
     def metadata(self, id: str) -> dict[str, Any]:
         # print(self.__class__.__name__ + "::" + sys._getframe().f_code.co_name)
         url = self.ENTRYPOINT + id + "/metadata/"

mdrsclient/api/laboratories.py

@@ -3,13 +3,14 @@ from typing import Final
 from pydantic import TypeAdapter
 
 from mdrsclient.api.base import BaseApi
-from mdrsclient.api.utils import token_check
+from mdrsclient.api.utils import retry_on_expired_token, token_check
 from mdrsclient.models import Laboratories, Laboratory
 
 
 class LaboratoriesApi(BaseApi):
     ENTRYPOINT: Final[str] = "v3/laboratories/"
 
+    @retry_on_expired_token
     def list(self) -> Laboratories:
         # print(self.__class__.__name__ + "::" + sys._getframe().f_code.co_name)
         url = self.ENTRYPOINT

mdrsclient/api/users.py

@@ -8,6 +8,10 @@ from mdrsclient.api.base import BaseApi
 from mdrsclient.exceptions import UnauthorizedException
 from mdrsclient.models import Token, User
 
+# (connect, read) seconds for the token refresh. Uploads are served by a separate
+# instance, so this one is not queued behind them and has no reason to be slow.
+TOKEN_REFRESH_TIMEOUT: Final[tuple[float, float]] = (5.0, 30.0)
+
 
 @dataclass(frozen=True)
 class UsersCurrentResponseLaboratory:
@@ -61,7 +65,10 @@ class UsersApi(BaseApi):
         # print(self.__class__.__name__ + "::" + sys._getframe().f_code.co_name)
         url = self.ENTRYPOINT + "token/refresh/"
         data: dict[str, str | int] = {"refresh": token.refresh}
-        response = self.connection.post(url, data=data)
+        # Bounded on its own: the caller holds a lock that spans processes while this runs,
+        # so a provider that accepts the connection and then goes quiet would otherwise
+        # stall every other request on this machine rather than just this one.
+        response = self.connection.post(url, data=data, timeout=TOKEN_REFRESH_TIMEOUT)
         if response.status_code == requests.codes.unauthorized:
             raise UnauthorizedException("Token is invalid or expired.")
         self._raise_response_error(response)

mdrsclient/api/utils.py

@@ -1,19 +1,124 @@
+import functools
+from typing import Any, Callable, TypeVar, cast
+
 from mdrsclient.api.users import UsersApi
 from mdrsclient.connection import MDRSConnection
-from mdrsclient.exceptions import UnauthorizedException
+from mdrsclient.exceptions import (
+    MDRSException,
+    ServerBusyException,
+    TokenExpiredException,
+    UnauthorizedException,
+)
+from mdrsclient.models import Token
+
+F = TypeVar("F", bound=Callable[..., Any])
+
+SERVER_BUSY_MESSAGE = (
+    "The server took too long to start handling the request and may be overloaded. "
+    "Try again, or reduce the number of parallel transfers."
+)
 
 
 def token_check(connection: MDRSConnection) -> None:
-    try:
-        connection.lock.acquire()
-        if connection.token is not None:
-            if connection.token.is_refresh_required:
-                user_api = UsersApi(connection)
-                try:
-                    connection.token = user_api.tokenRefresh(connection.token)
-                except UnauthorizedException:
-                    connection.logout()
-            elif connection.token.is_expired:
+    """
+    Bring the access token up to date before a request goes out.
+
+    Refreshing is a read-modify-write over a cache shared with every other client
+    process on this machine, and a rotating provider stops honouring the refresh token
+    it replaces. Two processes reaching this at once would otherwise both send the same
+    token, and the loser would be left holding one the server no longer accepts, so the
+    whole sequence runs under a lock that spans processes and the cache is re-read
+    inside it.
+    """
+    with connection.lock:
+        token = connection.token
+        if token is None or not (token.is_refresh_required or token.is_expired):
+            # Nothing to do, which is the answer for almost every request. The lock below
+            # reaches across processes and is held for a round trip, so it is worth
+            # knowing that before taking it.
+            return
+        with connection.cache_lock():
+            connection.reload_cache()
+            token = connection.token
+            if token is None:
+                return
+            if token.is_expired:
                 connection.logout()
-    finally:
-        connection.lock.release()
+                return
+            if not token.is_refresh_required:
+                return
+            user_api = UsersApi(connection)
+            try:
+                connection.token = user_api.tokenRefresh(token)
+            except UnauthorizedException:
+                connection.logout()
+
+
+def token_recover(connection: MDRSConnection, used: Token | None) -> bool:
+    """
+    Get a usable access token after one was refused for having expired.
+
+    A request can sit in the server's queue for longer than its access token lives, and
+    while it waited another thread or process may already have refreshed. Prefer what
+    they left behind: refreshing again would spend a round trip, and with a provider
+    that rotates refresh tokens it would retire one that is still in use.
+
+    Returns whether the caller now holds a token worth retrying with.
+    """
+    with connection.lock, connection.cache_lock():
+        connection.reload_cache()
+        token = connection.token
+        if token is None:
+            return False
+        if used is None or token != used:
+            # Somebody else has been here since the request went out. Assign rather than
+            # just read: the setter is what rewrites the session's Authorization header,
+            # so a plain reload would leave the retry carrying the refused token.
+            connection.token = token
+            return True
+        user_api = UsersApi(connection)
+        try:
+            connection.token = user_api.tokenRefresh(token)
+        except UnauthorizedException:
+            connection.logout()
+            return False
+        return True
+
+
+def retry_on_expired_token(func: F) -> F:
+    """
+    Send a request again once when the server found its access token expired.
+
+    The token is checked before every request, so this only happens when the request was
+    held long enough on the way in for a valid token to lapse - a queue behind uploads
+    that take minutes, most often.
+    """
+
+    @functools.wraps(func)
+    def wrapper(self: Any, *args: Any, **kwargs: Any) -> Any:
+        connection: MDRSConnection = self.connection
+        # Settle the token first so `used` is what the request actually carries. The
+        # wrapped method checks it again, which costs nothing once it is already current.
+        token_check(connection)
+        used = connection.token
+        try:
+            return func(self, *args, **kwargs)
+        except TokenExpiredException:
+            try:
+                recovered = token_recover(connection, used)
+            except MDRSException:
+                raise
+            except Exception as e:
+                # A busy server queues the refresh too, and failing here would report an
+                # endpoint the user never asked for rather than the reason.
+                raise ServerBusyException(SERVER_BUSY_MESSAGE) from e
+            if not recovered:
+                raise
+        try:
+            return func(self, *args, **kwargs)
+        except TokenExpiredException as e:
+            # A second refusal, with a token that was current when it left. The session is
+            # fine; the server is not starting requests before their credentials lapse.
+            raise ServerBusyException(SERVER_BUSY_MESSAGE) from e
+
+    return cast(F, wrapper)

mdrsclient/cache.py

@@ -1,8 +1,12 @@
+import contextlib
 import dataclasses
 import hashlib
 import json
 import os
-from typing import Protocol, runtime_checkable
+import tempfile
+import time
+from contextlib import AbstractContextManager
+from typing import Iterator, Protocol, runtime_checkable
 
 from pydantic import TypeAdapter, ValidationError
 from pydantic.dataclasses import dataclass
@@ -46,6 +50,19 @@ class CacheData:
 
 @runtime_checkable
 class CacheInterface(Protocol):
+    def lock(self) -> AbstractContextManager[None]:
+        """
+        Hold exclusive access to the cache for the duration of the block.
+
+        Refreshing a token is a read-modify-write, and the cache is shared with every
+        other client process using the same remote.
+        """
+        ...
+
+    def reload(self) -> None:
+        """Re-read the cache, discarding anything held from an earlier read."""
+        ...
+
     @property
     def token(self) -> Token | None: ...
     @token.setter
@@ -70,6 +87,14 @@ class InMemoryCache(CacheInterface):
     def __init__(self) -> None:
         self.__data = CacheData()
 
+    @contextlib.contextmanager
+    def lock(self) -> Iterator[None]:
+        # Nothing else can reach this cache, so there is nothing to exclude.
+        yield
+
+    def reload(self) -> None:
+        pass
+
     @property
     def token(self) -> Token | None:
         return self.__data.token
@@ -109,14 +134,46 @@ class CacheFile(CacheInterface):
     __serial: int
     __cache_dir: str
     __cache_file: str
+    __lock_file: str
+    __lock_depth: int
     __data: CacheData
 
     def __init__(self, remote: str) -> None:
         self.__serial = -1
         self.__cache_dir = os.path.join(CONFIG_DIRNAME, "cache")
         self.__cache_file = os.path.join(self.__cache_dir, remote + ".json")
+        self.__lock_file = os.path.join(self.__cache_dir, remote + ".lock")
+        self.__lock_depth = 0
         self.__data = CacheData()
 
+    @contextlib.contextmanager
+    def lock(self) -> Iterator[None]:
+        # Re-entrant, because every write takes it and a refresh is a write made while
+        # already holding it. A second flock on the same file from the same process would
+        # wait for a release that cannot come.
+        if self.__lock_depth > 0:
+            self.__lock_depth += 1
+            try:
+                yield
+            finally:
+                self.__lock_depth -= 1
+            return
+        # A separate file, so that replacing the cache cannot disturb the lock holders.
+        self.__ensure_cache_dir()
+        with open(self.__lock_file, "a") as f:
+            FileLock.lock(f)
+            self.__lock_depth = 1
+            try:
+                yield
+            finally:
+                self.__lock_depth = 0
+                FileLock.unlock(f)
+        os.chmod(self.__lock_file, 0o600)
+
+    def reload(self) -> None:
+        self.__serial = -1
+        self.__load()
+
     @property
     def token(self) -> Token | None:
         self.__load()
@@ -124,9 +181,10 @@ class CacheFile(CacheInterface):
 
     @token.setter
     def token(self, token: Token) -> None:
-        self.__load()
-        self.__data.token = token
-        self.__save()
+        with self.lock():
+            self.reload()
+            self.__data.token = token
+            self.__save()
 
     @token.deleter
     def token(self) -> None:
@@ -139,9 +197,10 @@ class CacheFile(CacheInterface):
 
     @user.setter
     def user(self, user: User) -> None:
-        self.__load()
-        self.__data.user = user
-        self.__save()
+        with self.lock():
+            self.reload()
+            self.__data.user = user
+            self.__save()
 
     @user.deleter
     def user(self) -> None:
@@ -154,9 +213,10 @@ class CacheFile(CacheInterface):
 
     @laboratories.setter
     def laboratories(self, laboratories: Laboratories) -> None:
-        self.__load()
-        self.__data.laboratories = laboratories
-        self.__save()
+        with self.lock():
+            self.reload()
+            self.__data.laboratories = laboratories
+            self.__save()
 
     def __clear(self) -> None:
         self.__data.clear()
@@ -164,8 +224,7 @@ class CacheFile(CacheInterface):
 
     def __load(self) -> None:
         if os.path.isfile(self.__cache_file):
-            stat = os.stat(self.__cache_file)
-            serial = hash((stat.st_uid, stat.st_gid, stat.st_mode, stat.st_size, stat.st_mtime))
+            serial = self.__stat_serial()
             if self.__serial != serial:
                 try:
                     with open(self.__cache_file) as f:
@@ -185,15 +244,44 @@ class CacheFile(CacheInterface):
 
     def __save(self) -> None:
         self.__ensure_cache_dir()
-        with open(self.__cache_file, "w") as f:
-            FileLock.lock(f)
-            self.__data.update_digest()
-            f.write(json.dumps(dataclasses.asdict(self.__data)))
-            FileLock.unlock(f)
+        self.__data.update_digest()
+        payload = json.dumps(dataclasses.asdict(self.__data))
+        # Written aside and moved into place: opening the cache for writing truncates it
+        # first, and a reader landing in that window would find the file empty and treat
+        # the session as broken.
+        fd, tmp_file = tempfile.mkstemp(dir=self.__cache_dir, prefix=".tmp-")
+        try:
+            with os.fdopen(fd, "w") as f:
+                f.write(payload)
+            # ensure file is secure.
+            os.chmod(tmp_file, 0o600)
+            self.__replace(tmp_file, self.__cache_file)
+        except BaseException:
+            if os.path.exists(tmp_file):
+                os.unlink(tmp_file)
+            raise
+        self.__serial = self.__stat_serial()
+
+    @staticmethod
+    def __replace(source: str, destination: str) -> None:
+        # Windows refuses the rename while another process still has the destination
+        # open, which a reader briefly does, so give it a moment rather than failing the
+        # save outright. On POSIX the rename always succeeds and the loop ends at once.
+        deadline = time.monotonic() + 5.0
+        while True:
+            try:
+                os.replace(source, destination)
+                return
+            except PermissionError:
+                if time.monotonic() >= deadline:
+                    raise
+                time.sleep(0.05)
+
+    def __stat_serial(self) -> int:
         stat = os.stat(self.__cache_file)
-        self.__serial = hash((stat.st_uid, stat.st_gid, stat.st_mode, stat.st_size, stat.st_mtime))
-        # ensure file is secure.
-        os.chmod(self.__cache_file, 0o600)
+        # st_ino and st_mtime_ns both move when the file is replaced, which a refresh
+        # that happens to produce the same number of bytes otherwise would not show.
+        return hash((stat.st_ino, stat.st_uid, stat.st_gid, stat.st_mode, stat.st_size, stat.st_mtime_ns))
 
     def __ensure_cache_dir(self) -> None:
         if not os.path.exists(self.__cache_dir):

mdrsclient/commands/ls.py

@@ -7,6 +7,7 @@ from pydantic.dataclasses import dataclass
 from mdrsclient.api import FilesApi, FoldersApi
 from mdrsclient.client import MdrsClient
 from mdrsclient.commands.base import BaseCommand
+from mdrsclient.config import build_download_url
 from mdrsclient.exceptions import UnauthorizedException
 from mdrsclient.models import File, Folder, FolderSimple, Laboratory
 
@@ -211,7 +212,7 @@ class LsCommand(BaseCommand):
             # "thumbnail": file.thumbnail,
             "description": file.description,
             "metadata": file.metadata,
-            "download_url": f"{context.client.connection.url}/{file.download_url}",
+            "download_url": build_download_url(context.client.connection.url, file.download_url),
             "created_at": file.created_at,
             "updated_at": file.updated_at,
         }

mdrsclient/config.py

@@ -10,6 +10,31 @@ from mdrsclient.settings import CONFIG_DIRNAME
 from mdrsclient.utils import FileLock
 
 
+def normalize_url(url: str) -> str:
+    """
+    Check a remote URL and put it in the one form every client agrees on.
+
+    `simple_host` is what lets a bare hostname through, so a development server on
+    `localhost` is as acceptable as a deployment behind a domain name. The trailing
+    slash goes because the URL is joined with a path that brings its own.
+    """
+    if not validators.url(url, simple_host=True, validate_scheme=lambda scheme: scheme in ("http", "https")):
+        raise IllegalArgumentException("malformed URI sequence")
+    return url.rstrip("/")
+
+
+def build_download_url(base_url: str | None, path: str) -> str:
+    """
+    Join a remote base URL with a path the API returned.
+
+    The API answers with a relative path and no leading separator, and a configuration
+    written before the URL was normalised may still carry a trailing one.
+    """
+    if path.startswith(("http://", "https://")):
+        return path
+    return f"{(base_url or '').rstrip('/')}/{path.lstrip('/')}"
+
+
 @runtime_checkable
 class ConfigInterface(Protocol):
     remote: str
@@ -42,8 +67,7 @@ class InMemoryConfig(ConfigInterface):
 
     @url.setter
     def url(self, url: str) -> None:
-        if not validators.url(url):
-            raise IllegalArgumentException("malformed URI sequence")
+        url = normalize_url(url)
         with self.__lock:
             self.__configs[self.remote] = url
 
@@ -91,8 +115,7 @@ class ConfigFile(ConfigInterface):
 
     @url.setter
     def url(self, url: str) -> None:
-        if not validators.url(url):
-            raise IllegalArgumentException("malformed URI sequence")
+        url = normalize_url(url)
         self.__load()
         if self.__config.has_section(self.remote):
             self.__config.remove_section(self.remote)

mdrsclient/connection.py

@@ -1,5 +1,6 @@
 import platform
 import threading
+from contextlib import AbstractContextManager
 from typing import TypedDict
 
 from requests import Response, Session
@@ -23,6 +24,7 @@ class _KwArgsMDRSConnectionPost(TypedDict, total=False):
     params: dict[str, str | int]
     data: dict[str, str | int] | MultipartEncoder
     headers: dict[str, str]
+    timeout: float | tuple[float, float]
 
 
 class _KwArgsMDRSConnectionPut(TypedDict, total=False):
@@ -61,6 +63,14 @@ class MDRSConnection:
     def delete(self, url: str, **kwargs: Unpack[_KwArgsMDRSConnectionDelete]) -> Response:
         return self.session.delete(self.__build_url(url), **kwargs)
 
+    def cache_lock(self) -> AbstractContextManager[None]:
+        """Hold exclusive access to the login cache across every process using it."""
+        return self.__cache.lock()
+
+    def reload_cache(self) -> None:
+        """Re-read the login cache, discarding anything held from an earlier read."""
+        self.__cache.reload()
+
     def logout(self) -> None:
         del self.__cache.user
         del self.__cache.token
@@ -96,7 +106,9 @@ class MDRSConnection:
             return path
         if self.url == "":
             raise MissingConfigurationException("remote host is not configured")
-        return f"{self.url}/{path}"
+        # The path brings its own separator, and a configuration written before the URL
+        # was normalised may still carry a trailing slash of its own.
+        return f"{self.url.rstrip('/')}/{path}"
 
     def __prepare_headers(self) -> None:
         self.session.headers.update(

mdrsclient/exceptions.py

@@ -28,6 +28,18 @@ class UnauthorizedException(MDRSException):
     pass
 
 
+class TokenExpiredException(UnauthorizedException):
+    """Thrown when the access token was still valid when sent but had expired on arrival"""
+
+    pass
+
+
+class ServerBusyException(MDRSException):
+    """Thrown when the server did not start handling a request before its token lapsed"""
+
+    pass
+
+
 class ForbiddenException(MDRSException):
     """Thrown when the current user does not have enough privileges to access the resource"""
 

mdrsclient/transfer.py

@@ -6,7 +6,7 @@ from unicodedata import normalize
 from pydantic.dataclasses import dataclass
 
 from mdrsclient.api import FilesApi, FoldersApi
-from mdrsclient.exceptions import IllegalArgumentException, MDRSException, UnexpectedException
+from mdrsclient.exceptions import IllegalArgumentException, UnexpectedException
 from mdrsclient.models import File, Folder, Laboratory
 from mdrsclient.models.file import find_file
 from mdrsclient.settings import CONCURRENT
@@ -27,7 +27,6 @@ class DownloadFileInfo:
 
 @dataclass
 class DownloadContext:
-    hasError: bool
     isSkipIfExists: bool
     files: list[DownloadFileInfo]
 
@@ -47,54 +46,67 @@ class Uploader:
         laboratory = self.client.find_laboratory(laboratory_name)
         folder = self.client.find_folder(laboratory, r_path)
         files = self.client.find_files(folder.id)
-        infos: list[UploadFileInfo] = []
         if os.path.isdir(l_path):
             if not is_recursive:
                 raise IllegalArgumentException(f"Cannot upload `{local_path}`: Is a directory.")
-            folder_api = FoldersApi(self.client.connection)
-            folder_map: dict[str, Folder] = {}
-            folder_map[r_path] = folder
-            files_map: dict[str, list[File]] = {}
-            files_map[r_path] = files
-            l_basename = os.path.basename(l_path)
-            for dirpath, _, filenames in os.walk(l_path, followlinks=True):
-                sub = l_basename if dirpath == l_path else os.path.join(l_basename, os.path.relpath(dirpath, l_path))
-                d_dirname = os.path.join(r_path, sub)
-                d_basename = os.path.basename(d_dirname)
-                # prepare destination parent path
-                d_parent_dirname = os.path.dirname(d_dirname)
-                if folder_map.get(d_parent_dirname) is None:
-                    parent_folder = self.client.find_folder(laboratory, d_parent_dirname)
-                    folder_map[d_parent_dirname] = parent_folder
-                    parent_files = self.client.find_files(parent_folder.id)
-                    files_map[d_parent_dirname] = parent_files
-                # prepare destination path
-                if folder_map.get(d_dirname) is None:
-                    d_folder = folder_map[d_parent_dirname].find_sub_folder(d_basename)
-                    if d_folder is None:
-                        d_folder_id = folder_api.create(normalize("NFC", d_basename), folder_map[d_parent_dirname].id)
-                    else:
-                        d_folder_id = d_folder.id
-                    print(d_dirname)
-                    folder_map[d_dirname] = folder_api.retrieve(d_folder_id)
-                    files_map[d_dirname] = self.client.find_files(d_folder_id)
-                    if d_folder is None:
-                        folder_map[d_parent_dirname].sub_folders.append(folder_map[d_dirname])
-                # register upload file list
-                for filename in filenames:
-                    infos.append(
-                        UploadFileInfo(folder_map[d_dirname], files_map[d_dirname], os.path.join(dirpath, filename))
-                    )
+            infos = self.__collect_directory_uploads(laboratory, r_path, l_path, folder, files)
         else:
-            infos.append(UploadFileInfo(folder, files, l_path))
-        self.__multiple_upload(infos, is_skip_if_exists)
+            infos = [UploadFileInfo(folder, files, l_path)]
+        if not self.__multiple_upload(infos, is_skip_if_exists):
+            # One file failing is worth reporting on its own line, and worth the caller
+            # hearing about: a batch that lost files is not a batch that succeeded.
+            raise UnexpectedException("Some files failed to upload.")
 
-    def __multiple_upload(self, infos: list[UploadFileInfo], is_skip_if_exists: bool) -> None:
+    def __collect_directory_uploads(
+        self, laboratory: Laboratory, r_path: str, l_path: str, folder: Folder, files: list[File]
+    ) -> list[UploadFileInfo]:
+        """Mirror a local directory tree on the remote, and list the files to send into it."""
+        infos: list[UploadFileInfo] = []
+        folder_api = FoldersApi(self.client.connection)
+        folder_map: dict[str, Folder] = {}
+        folder_map[r_path] = folder
+        files_map: dict[str, list[File]] = {}
+        files_map[r_path] = files
+        l_basename = os.path.basename(l_path)
+        for dirpath, _, filenames in os.walk(l_path, followlinks=True):
+            sub = l_basename if dirpath == l_path else os.path.join(l_basename, os.path.relpath(dirpath, l_path))
+            d_dirname = os.path.join(r_path, sub)
+            d_basename = os.path.basename(d_dirname)
+            # prepare destination parent path
+            d_parent_dirname = os.path.dirname(d_dirname)
+            if folder_map.get(d_parent_dirname) is None:
+                parent_folder = self.client.find_folder(laboratory, d_parent_dirname)
+                folder_map[d_parent_dirname] = parent_folder
+                parent_files = self.client.find_files(parent_folder.id)
+                files_map[d_parent_dirname] = parent_files
+            # prepare destination path
+            if folder_map.get(d_dirname) is None:
+                d_folder = folder_map[d_parent_dirname].find_sub_folder(d_basename)
+                if d_folder is None:
+                    d_folder_id = folder_api.create(normalize("NFC", d_basename), folder_map[d_parent_dirname].id)
+                else:
+                    d_folder_id = d_folder.id
+                print(d_dirname)
+                folder_map[d_dirname] = folder_api.retrieve(d_folder_id)
+                files_map[d_dirname] = self.client.find_files(d_folder_id)
+                if d_folder is None:
+                    folder_map[d_parent_dirname].sub_folders.append(folder_map[d_dirname])
+            # register upload file list
+            for filename in filenames:
+                infos.append(
+                    UploadFileInfo(folder_map[d_dirname], files_map[d_dirname], os.path.join(dirpath, filename))
+                )
+        return infos
+
+    def __multiple_upload(self, infos: list[UploadFileInfo], is_skip_if_exists: bool) -> bool:
+        """Send every file, and report whether all of them arrived."""
         file_api = FilesApi(self.client.connection)
         with ThreadPoolExecutor(max_workers=CONCURRENT) as pool:
-            pool.map(lambda x: self.__multiple_upload_worker(file_api, x, is_skip_if_exists), infos)
+            results = pool.map(lambda x: self.__multiple_upload_worker(file_api, x, is_skip_if_exists), infos)
+            # Consumed inside the block: the results are what carry each worker's verdict.
+            return all(list(results))
 
-    def __multiple_upload_worker(self, file_api: FilesApi, info: UploadFileInfo, is_skip_if_exists: bool) -> None:
+    def __multiple_upload_worker(self, file_api: FilesApi, info: UploadFileInfo, is_skip_if_exists: bool) -> bool:
         basename = os.path.basename(info.path)
         file = find_file(info.files, basename)
         try:
@@ -103,8 +115,14 @@ class Uploader:
             elif not is_skip_if_exists or file.size != os.path.getsize(info.path):
                 file_api.update(file, info.path)
             print(os.path.join(info.folder.path, basename))
-        except MDRSException as e:
-            print(f"Error: {e}")
+        except Exception as e:
+            # Everything, not just the exceptions the API layer raises: the batch verdict
+            # is read now that the results are consumed, and a file vanishing between the
+            # walk and the upload would otherwise end the whole run with a traceback and
+            # throw away what every other file did.
+            print(f"Failed: {info.path}: {e}")
+            return False
+        return True
 
 
 class Downloader:
@@ -120,6 +138,21 @@ class Downloader:
         password: str | None = None,
         excludes: list[str] | None = None,
     ) -> None:
+        if not self.__download(remote_path, local_path, is_recursive, is_skip_if_exists, password, excludes):
+            # Every failure has already been printed against the file it belongs to.
+            # This is what makes the command as a whole end in failure.
+            raise UnexpectedException("Some files failed to download.")
+
+    def __download(
+        self,
+        remote_path: str,
+        local_path: str,
+        is_recursive: bool,
+        is_skip_if_exists: bool,
+        password: str | None,
+        excludes: list[str] | None,
+    ) -> bool:
+        """Fetch what the remote path names, and report whether every file arrived."""
         excludes_clean = excludes or []
         # Detect DOI path: "remote:10.xxxx/prefix.ID[/optional/sub/path]"
         path_component = remote_path.split(":", 1)[1] if ":" in remote_path else ""
@@ -144,12 +177,11 @@ class Downloader:
                 file = find_file(r_parent_files, r_basename)
                 if file is not None:
                     if self.__check_excludes(excludes_clean, laboratory, r_parent_folder, file):
-                        return
-                    context = DownloadContext(False, is_skip_if_exists, [])
+                        return True
+                    context = DownloadContext(is_skip_if_exists, [])
                     l_path = os.path.join(l_dirname, r_basename)
                     context.files.append(DownloadFileInfo(file, l_path))
-                    self.__multiple_download(context)
-                    return
+                    return self.__multiple_download(context)
                 else:
                     folder_simple = r_parent_folder.find_sub_folder(r_basename)
                     if folder_simple is None:
@@ -161,19 +193,17 @@ class Downloader:
             if not is_recursive:
                 # Non-recursive: download only the files at the top level of the DOI folder.
                 files = self.client.find_files(folder.id)
-                context = DownloadContext(False, is_skip_if_exists, [])
+                context = DownloadContext(is_skip_if_exists, [])
                 for file in files:
                     if self.__check_excludes(excludes_clean, laboratory, folder, file):
                         continue
                     l_path = os.path.join(l_dirname, file.name)
                     context.files.append(DownloadFileInfo(file, l_path))
-                self.__multiple_download(context)
-                return
+                return self.__multiple_download(context)
             folder_api = FoldersApi(self.client.connection)
-            self.__multiple_download_pickup_recursive_files(
+            return self.__multiple_download_pickup_recursive_files(
                 folder_api, laboratory, folder.id, l_dirname, excludes_clean, is_skip_if_exists
             )
-            return
 
         remote, laboratory_name, r_path = self.client.parse_remote_host_with_path(remote_path)
         r_path = r_path.rstrip("/")
@@ -189,11 +219,11 @@ class Downloader:
         file = find_file(r_parent_files, r_basename)
         if file is not None:
             if self.__check_excludes(excludes_clean, laboratory, r_parent_folder, file):
-                return
-            context = DownloadContext(False, is_skip_if_exists, [])
+                return True
+            context = DownloadContext(is_skip_if_exists, [])
             l_path = os.path.join(l_dirname, r_basename)
             context.files.append(DownloadFileInfo(file, l_path))
-            self.__multiple_download(context)
+            return self.__multiple_download(context)
         else:
             folder = r_parent_folder.find_sub_folder(r_basename)
             if folder is None:
@@ -201,7 +231,7 @@ class Downloader:
             if not is_recursive:
                 raise IllegalArgumentException(f"Cannot download `{r_path}`: Is a folder.")
             folder_api = FoldersApi(self.client.connection)
-            self.__multiple_download_pickup_recursive_files(
+            return self.__multiple_download_pickup_recursive_files(
                 folder_api, laboratory, folder.id, l_dirname, excludes_clean, is_skip_if_exists
             )
 
@@ -213,47 +243,60 @@ class Downloader:
         basedir: str,
         excludes: list[str],
         is_skip_if_exists: bool,
-    ) -> None:
-        context = DownloadContext(False, is_skip_if_exists, [])
-        folder = folder_api.retrieve(folder_id)
-        files = self.client.find_files(folder.id)
+    ) -> bool:
+        context = DownloadContext(is_skip_if_exists, [])
+        try:
+            folder = folder_api.retrieve(folder_id)
+            files = self.client.find_files(folder.id)
+        except Exception as e:
+            print(f"Failed: {basedir}: {e}")
+            return False
         dirname = os.path.join(basedir, folder.name)
         if self.__check_excludes(excludes, laboratory, folder, None):
-            return
-        if not os.path.exists(dirname):
-            os.makedirs(dirname)
+            return True
+        try:
+            # `exist_ok` rather than a prior check: two workers can reach the same parent.
+            os.makedirs(dirname, exist_ok=True)
+        except OSError as e:
+            # One folder the client cannot make locally is not a reason to abandon its
+            # siblings, which is what this walk now promises.
+            print(f"Failed: {dirname}: {e}")
+            return False
         print(dirname)
         for file in files:
             if self.__check_excludes(excludes, laboratory, folder, file):
                 continue
             path = os.path.join(dirname, file.name)
             context.files.append(DownloadFileInfo(file, path))
-        self.__multiple_download(context)
-        if context.hasError:
-            raise UnexpectedException("Some files failed to download.")
+        succeeded = self.__multiple_download(context)
+        # A folder that lost a file is still a folder whose sub-folders the user asked
+        # for, so the walk carries on and the verdict is collected for the caller.
         for sub_folder in folder.sub_folders:
-            self.__multiple_download_pickup_recursive_files(
+            if not self.__multiple_download_pickup_recursive_files(
                 folder_api, laboratory, sub_folder.id, dirname, excludes, is_skip_if_exists
-            )
+            ):
+                succeeded = False
+        return succeeded
 
-    def __multiple_download(self, context: DownloadContext) -> None:
+    def __multiple_download(self, context: DownloadContext) -> bool:
+        """Fetch every file in the batch, and report whether all of them arrived."""
         file_api = FilesApi(self.client.connection)
         with ThreadPoolExecutor(max_workers=CONCURRENT) as pool:
             results = pool.map(
                 lambda x: self.__multiple_download_worker(file_api, x, context.isSkipIfExists), context.files
             )
-            hasError = next(filter(lambda x: x is False, results), None)
-            if hasError is not None:
-                context.hasError = True
+            # Consumed inside the block, and in full: every worker's verdict counts, not
+            # just the first refusal.
+            return all(list(results))
 
     def __multiple_download_worker(self, file_api: FilesApi, info: DownloadFileInfo, is_skip_if_exists: bool) -> bool:
         if not is_skip_if_exists or not os.path.exists(info.path) or info.file.size != os.path.getsize(info.path):
             try:
                 file_api.download(info.file, info.path)
-            except Exception:
-                print(f"Failed: {info.path}")
-                if os.path.isfile(info.path):
-                    os.remove(info.path)
+            except Exception as e:
+                # Nothing to clear up: a failed transfer writes only to its own scratch
+                # file beside the destination, and removes that itself.
+                print(f"Failed: {info.path}: {e}")
                 return False
         print(info.path)
         return True

mdrsclient/utils.py

@@ -1,5 +1,6 @@
 import os
-from typing import IO, Any
+import time
+from typing import IO, Any, Final
 from urllib.parse import parse_qs, urlparse
 
 if os.name == "nt":
@@ -9,10 +10,23 @@ elif os.name == "posix":
 
 
 class FileLock:
+    # Long enough to outlast a token refresh, which is what the lock is held across.
+    WAIT_SECONDS: Final[float] = 60.0
+
     @staticmethod
     def lock(file: IO[Any]) -> None:
         if os.name == "nt":
-            msvcrt.locking(file.fileno(), msvcrt.LK_LOCK, 1)
+            # msvcrt.LK_LOCK gives up after ten one-second attempts, which is shorter
+            # than the refresh it now has to wait for, so do the waiting here instead.
+            deadline = time.monotonic() + FileLock.WAIT_SECONDS
+            while True:
+                try:
+                    msvcrt.locking(file.fileno(), msvcrt.LK_NBLCK, 1)
+                    return
+                except OSError:
+                    if time.monotonic() >= deadline:
+                        raise
+                    time.sleep(0.1)
         elif os.name == "posix":
             fcntl.flock(file.fileno(), fcntl.LOCK_EX)
 

pyproject.toml

@@ -1,11 +1,21 @@
-[tool.poetry]
+[build-system]
+requires = ["flit_core>=3.12.0,<4.0.0"]
+build-backend = "flit_core.buildapi"
+
+[tool.flit.module]
+name = "mdrsclient"
+
+[project]
 name = "mdrs-client-python"
 version = "1.3.18"
 description = "The mdrs-client-python is python library and a command-line client for up- and downloading files to and from MDRS based repository."
-authors = ["Yoshihiro OKUMURA <yoshihiro.okumura@riken.jp>"]
-license = "MIT"
+authors = [
+    { name = "Yoshihiro OKUMURA", email = "yoshihiro.okumura@riken.jp" }
+]
+license = { text = "MIT" }
 readme = "README.md"
-classifiers=[
+requires-python = ">=3.10"
+classifiers = [
     "Development Status :: 4 - Beta",
     "Environment :: Console",
     "Intended Audience :: Developers",
@@ -18,33 +28,28 @@ classifiers=[
     "OSI Approved :: MIT License",
     "Topic :: Utilities",
 ]
-packages = [
-    { include = "mdrsclient" }
+
+dependencies = [
+    "requests>=2.34.2",
+    "requests-toolbelt>=1.0.0",
+    "python-dotenv>=1.2.2",
+    "pydantic>=2.13.4",
+    "pydantic-settings>=2.14.2",
+    "PyJWT>=2.13.0",
+    "validators>=0.35.0",
 ]
 
-[tool.poetry.dependencies]
-python = "^3.10"
-requests = "^2.34.2"
-requests-toolbelt = "^1.0.0"
-python-dotenv = "^1.1.0"
-pydantic = "^2.13.4"
-pydantic-settings = "^2.14.2"
-PyJWT = "^2.13.0"
-validators = "^0.35.0"
+[project.optional-dependencies]
+dev = [
+    "black>=26.5.1",
+    "flake8>=7.3.0",
+    "Flake8-pyproject>=1.2.4",
+    "isort>=8.0.1",
+    "pyright>=1.1.411",
+]
 
-[tool.poetry.group.dev.dependencies]
-black = "^26.5.1"
-flake8 = "^7.2.0"
-Flake8-pyproject = "^1.2.3"
-isort = "^8.0.1"
-pyright = "^1.1.411"
-
-[tool.poetry.scripts]
-mdrs = 'mdrsclient.__main__:main'
-
-[build-system]
-requires = ["poetry-core"]
-build-backend = "poetry.core.masonry.api"
+[project.scripts]
+mdrs = "mdrsclient.__main__:main"
 
 [tool.black]
 line-length = 120

tests/test_token_cache.py

@@ -0,0 +1,198 @@
+import tempfile
+import time
+import unittest
+from unittest.mock import patch
+
+import jwt
+
+from mdrsclient.api.utils import retry_on_expired_token, token_check, token_recover
+from mdrsclient.cache import CacheFile
+from mdrsclient.config import normalize_url
+from mdrsclient.connection import MDRSConnection
+from mdrsclient.exceptions import IllegalArgumentException, ServerBusyException, TokenExpiredException
+from mdrsclient.models import Token
+
+REMOTE = "unittest"
+
+
+def make_token(access_offset: int, refresh_offset: int, label: str) -> Token:
+    now = int(time.time())
+
+    def encode(token_type: str, offset: int) -> str:
+        return jwt.encode(
+            {
+                "token_type": token_type,
+                "exp": now + offset,
+                "iat": now,
+                "jti": f"{label}-{token_type}",
+                "user_id": 1,
+            },
+            "unittest-signing-key-not-verified-anywhere",
+        )
+
+    return Token(access=encode("access", access_offset), refresh=encode("refresh", refresh_offset))
+
+
+class TestTokenCache(unittest.TestCase):
+    def setUp(self) -> None:
+        self.tmp = tempfile.TemporaryDirectory()
+        patcher = patch("mdrsclient.cache.CONFIG_DIRNAME", self.tmp.name)
+        patcher.start()
+        self.addCleanup(patcher.stop)
+        self.addCleanup(self.tmp.cleanup)
+
+    def test_reload_picks_up_a_write_from_another_holder(self) -> None:
+        """The cache is shared, so a value read earlier can already be out of date."""
+        reader = CacheFile(REMOTE)
+        writer = CacheFile(REMOTE)
+
+        first = make_token(3600, 86400, "first")
+        writer.token = first
+        self.assertEqual(reader.token, first)
+
+        second = make_token(3600, 86400, "second")
+        writer.token = second
+
+        reader.reload()
+        self.assertEqual(reader.token, second)
+
+    def test_token_check_uses_the_token_another_holder_just_wrote(self) -> None:
+        """A rotating provider drops the token it replaces, so the refresh must not
+        be sent again once someone else has already made the round trip."""
+        connection = MDRSConnection(REMOTE, "http://localhost:8000/api/")
+        connection.token = make_token(-60, 86400, "stale")
+
+        # Another process refreshes while this one is between requests.
+        other = CacheFile(REMOTE)
+        rotated = make_token(3600, 86400, "rotated")
+        other.token = rotated
+
+        with patch("mdrsclient.api.utils.UsersApi") as users_api:
+            token_check(connection)
+
+        users_api.assert_not_called()
+        self.assertEqual(connection.token, rotated)
+
+    def test_token_check_refreshes_when_nothing_else_has(self) -> None:
+        connection = MDRSConnection(REMOTE, "http://localhost:8000/api/")
+        connection.token = make_token(-60, 86400, "stale")
+        rotated = make_token(3600, 86400, "rotated")
+
+        with patch("mdrsclient.api.utils.UsersApi") as users_api:
+            users_api.return_value.tokenRefresh.return_value = rotated
+            token_check(connection)
+
+        users_api.return_value.tokenRefresh.assert_called_once()
+        self.assertEqual(connection.token, rotated)
+
+    def test_token_check_logs_out_once_the_refresh_token_expires(self) -> None:
+        connection = MDRSConnection(REMOTE, "http://localhost:8000/api/")
+        connection.token = make_token(-3600, -60, "dead")
+
+        with patch("mdrsclient.api.utils.UsersApi") as users_api:
+            token_check(connection)
+
+        users_api.assert_not_called()
+        self.assertIsNone(connection.token)
+
+
+class TestUrlNormalization(unittest.TestCase):
+    """Both clients share config.ini, so they have to agree on what a remote URL is."""
+
+    def test_trailing_slash_is_dropped(self) -> None:
+        self.assertEqual(normalize_url("http://127.0.0.1:8000/api/"), "http://127.0.0.1:8000/api")
+        self.assertEqual(normalize_url("https://neurodata.riken.jp/api/"), "https://neurodata.riken.jp/api")
+
+    def test_bare_hostname_is_accepted(self) -> None:
+        self.assertEqual(normalize_url("http://localhost:8000/api"), "http://localhost:8000/api")
+
+    def test_only_http_schemes_are_accepted(self) -> None:
+        for url in ("ftp://x.example.com/", "file:///etc/passwd", "not-a-url", "http://"):
+            with self.subTest(url=url), self.assertRaises(IllegalArgumentException):
+                normalize_url(url)
+
+
+class TestExpiredTokenRetry(unittest.TestCase):
+    """A request can wait in the server's queue for longer than its access token lives."""
+
+    def setUp(self) -> None:
+        self.tmp = tempfile.TemporaryDirectory()
+        patcher = patch("mdrsclient.cache.CONFIG_DIRNAME", self.tmp.name)
+        patcher.start()
+        self.addCleanup(patcher.stop)
+        self.addCleanup(self.tmp.cleanup)
+        self.connection = MDRSConnection(REMOTE, "http://localhost:8000/api")
+        self.connection.token = make_token(3600, 86400, "sent")
+
+    def test_a_token_someone_else_refreshed_is_reused_rather_than_replaced(self) -> None:
+        used = self.connection.token
+        rotated = make_token(3600, 86400, "rotated")
+        CacheFile(REMOTE).token = rotated  # another process, while the request waited
+
+        with patch("mdrsclient.api.utils.UsersApi") as users_api:
+            recovered = token_recover(self.connection, used)
+
+        self.assertTrue(recovered)
+        users_api.assert_not_called()
+        self.assertEqual(self.connection.token, rotated)
+        # The retry carries whatever the session header says, not what the cache holds,
+        # so reading the new token through is not enough on its own.
+        self.assertEqual(self.connection.session.headers["Authorization"], f"Bearer {rotated.access}")
+
+    def test_the_token_is_refreshed_when_nobody_else_has(self) -> None:
+        used = self.connection.token
+        rotated = make_token(3600, 86400, "rotated")
+
+        with patch("mdrsclient.api.utils.UsersApi") as users_api:
+            users_api.return_value.tokenRefresh.return_value = rotated
+            recovered = token_recover(self.connection, used)
+
+        self.assertTrue(recovered)
+        users_api.return_value.tokenRefresh.assert_called_once()
+        self.assertEqual(self.connection.token, rotated)
+
+    def test_the_request_is_sent_once_more_and_only_once(self) -> None:
+        calls: list[str] = []
+
+        class Api:
+            def __init__(self, connection: MDRSConnection) -> None:
+                self.connection = connection
+
+            @retry_on_expired_token
+            def send(self) -> str:
+                calls.append("sent")
+                if len(calls) == 1:
+                    raise TokenExpiredException("expired on arrival")
+                return "ok"
+
+        with patch("mdrsclient.api.utils.UsersApi") as users_api:
+            users_api.return_value.tokenRefresh.return_value = make_token(3600, 86400, "rotated")
+            self.assertEqual(Api(self.connection).send(), "ok")
+
+        self.assertEqual(len(calls), 2)
+
+    def test_a_second_refusal_is_reported_as_an_overloaded_server(self) -> None:
+        calls: list[str] = []
+
+        class Api:
+            def __init__(self, connection: MDRSConnection) -> None:
+                self.connection = connection
+
+            @retry_on_expired_token
+            def send(self) -> str:
+                calls.append("sent")
+                raise TokenExpiredException("expired on arrival")
+
+        with patch("mdrsclient.api.utils.UsersApi") as users_api:
+            users_api.return_value.tokenRefresh.return_value = make_token(3600, 86400, "rotated")
+            with self.assertRaises(ServerBusyException) as caught:
+                Api(self.connection).send()
+
+        # Sent twice and no more, and the message names the cause the user can act on
+        # rather than an authentication failure that would send them to the login form.
+        self.assertEqual(len(calls), 2)
+        self.assertIn("overloaded", str(caught.exception))
+
+
+if __name__ == "__main__":
+    unittest.main()

tests/test_transfer.py

@@ -0,0 +1,297 @@
+import os
+import tempfile
+import unittest
+from io import StringIO
+from unittest.mock import MagicMock, patch
+
+from mdrsclient.api import FilesApi
+from mdrsclient.exceptions import MDRSException, UnexpectedException
+from mdrsclient.models import File, Folder, FolderSimple, Laboratory
+from mdrsclient.transfer import Downloader, Uploader
+
+TIMESTAMP = "2026-01-01T00:00:00+09:00"
+
+
+def make_file(id: str, name: str, size: int = 1) -> File:
+    return File(
+        id=id,
+        name=name,
+        type="text/plain",
+        size=size,
+        thumbnail=None,
+        description="",
+        metadata={},
+        download_url=f"v3/files/{id}/download/",
+        created_at=TIMESTAMP,
+        updated_at=TIMESTAMP,
+    )
+
+
+def make_folder_simple(id: str, name: str) -> FolderSimple:
+    return FolderSimple(
+        id=id,
+        pid=None,
+        name=name,
+        access_level=1,
+        lock=False,
+        size=0,
+        laboratory_id=1,
+        description="",
+        created_at=TIMESTAMP,
+        updated_at=TIMESTAMP,
+        restrict_opened_at=None,
+    )
+
+
+def make_folder(id: str, name: str, path: str, sub_folders: list[FolderSimple] | None = None) -> Folder:
+    return Folder(
+        id=id,
+        pid=None,
+        name=name,
+        access_level=1,
+        lock=False,
+        size=0,
+        laboratory_id=1,
+        description="",
+        created_at=TIMESTAMP,
+        updated_at=TIMESTAMP,
+        restrict_opened_at=None,
+        metadata=[],
+        sub_folders=sub_folders if sub_folders is not None else [],
+        path=path,
+    )
+
+
+LABORATORY = Laboratory(id=1, name="mylab", pi_name="PI", full_name="My Laboratory")
+
+
+class TestUploadReportsFailure(unittest.TestCase):
+    """A file that never reached the server must not leave the command reporting success."""
+
+    def make_client(self, folder: Folder, files: list[File]) -> MagicMock:
+        client = MagicMock()
+        client.parse_remote_host_with_path.return_value = ("myremote", "mylab", "/")
+        client.find_laboratory.return_value = LABORATORY
+        client.find_folder.return_value = folder
+        client.find_files.return_value = files
+        return client
+
+    def test_a_refused_upload_is_raised_to_the_caller(self):
+        client = self.make_client(make_folder("f1", "root", "/"), [])
+        with tempfile.TemporaryDirectory() as tmp:
+            local = os.path.join(tmp, "data.txt")
+            with open(local, "w") as f:
+                f.write("x")
+            with patch("mdrsclient.transfer.FilesApi") as files_api_class:
+                files_api_class.return_value.create.side_effect = MDRSException("Access is denied.")
+                with self.assertRaises(UnexpectedException):
+                    Uploader(client).upload(local, "myremote:/mylab/", False, False)
+
+    def test_an_error_the_api_layer_did_not_raise_is_still_counted(self):
+        """A file that vanished between the walk and the upload must not end the run."""
+        client = self.make_client(make_folder("f1", "root", "/"), [])
+        with tempfile.TemporaryDirectory() as tmp:
+            local = os.path.join(tmp, "data.txt")
+            with open(local, "w") as f:
+                f.write("x")
+            with patch("mdrsclient.transfer.FilesApi") as files_api_class:
+                files_api_class.return_value.create.side_effect = FileNotFoundError(local)
+                with self.assertRaises(UnexpectedException):
+                    Uploader(client).upload(local, "myremote:/mylab/", False, False)
+
+    def test_a_failed_upload_names_the_file(self):
+        client = self.make_client(make_folder("f1", "root", "/"), [])
+        with tempfile.TemporaryDirectory() as tmp:
+            local = os.path.join(tmp, "data.txt")
+            with open(local, "w") as f:
+                f.write("x")
+            with patch("mdrsclient.transfer.FilesApi") as files_api_class:
+                files_api_class.return_value.create.side_effect = MDRSException("Access is denied.")
+                with patch("sys.stdout", new=StringIO()) as fake_out:
+                    with self.assertRaises(UnexpectedException):
+                        Uploader(client).upload(local, "myremote:/mylab/", False, False)
+                    self.assertIn(local, fake_out.getvalue())
+
+    def test_an_upload_that_worked_stays_quiet(self):
+        client = self.make_client(make_folder("f1", "root", "/"), [])
+        with tempfile.TemporaryDirectory() as tmp:
+            local = os.path.join(tmp, "data.txt")
+            with open(local, "w") as f:
+                f.write("x")
+            with patch("mdrsclient.transfer.FilesApi") as files_api_class:
+                files_api_class.return_value.create.return_value = "new-id"
+                Uploader(client).upload(local, "myremote:/mylab/", False, False)
+                files_api_class.return_value.create.assert_called_once()
+
+
+class TestDownloadReportsFailure(unittest.TestCase):
+    def make_client(self, remote_path_parts, folder: Folder, files_by_folder: dict[str, list[File]]) -> MagicMock:
+        client = MagicMock()
+        client.is_doi.return_value = False
+        client.parse_remote_host_with_path.return_value = remote_path_parts
+        client.find_laboratory.return_value = LABORATORY
+        client.find_folder.return_value = folder
+        client.find_files.side_effect = lambda folder_id: files_by_folder.get(folder_id, [])
+        return client
+
+    def test_a_single_file_that_failed_is_raised_to_the_caller(self):
+        parent = make_folder("p1", "root", "/")
+        client = self.make_client(("myremote", "mylab", "/data.txt"), parent, {"p1": [make_file("x1", "data.txt")]})
+        with tempfile.TemporaryDirectory() as tmp:
+            with patch("mdrsclient.transfer.FilesApi") as files_api_class:
+                files_api_class.return_value.download.side_effect = OSError("Permission denied.")
+                with self.assertRaises(UnexpectedException):
+                    Downloader(client).download("myremote:/mylab/data.txt", tmp)
+
+    def test_a_single_file_that_arrived_stays_quiet(self):
+        parent = make_folder("p1", "root", "/")
+        client = self.make_client(("myremote", "mylab", "/data.txt"), parent, {"p1": [make_file("x1", "data.txt")]})
+        with tempfile.TemporaryDirectory() as tmp:
+            with patch("mdrsclient.transfer.FilesApi") as files_api_class:
+                files_api_class.return_value.download.return_value = True
+                Downloader(client).download("myremote:/mylab/data.txt", tmp)
+                files_api_class.return_value.download.assert_called_once()
+
+    def test_a_failed_file_does_not_abandon_the_remaining_sub_folders(self):
+        parent = make_folder("p1", "lab", "/", [make_folder_simple("f1", "root")])
+        folders = {
+            "f1": make_folder("f1", "root", "/root/", [make_folder_simple("fa", "a"), make_folder_simple("fb", "b")]),
+            "fa": make_folder("fa", "a", "/root/a/"),
+            "fb": make_folder("fb", "b", "/root/b/"),
+        }
+        files_by_folder = {
+            "p1": [],
+            "f1": [make_file("bad", "bad.txt")],
+            "fa": [make_file("good1", "good1.txt")],
+            "fb": [make_file("good2", "good2.txt")],
+        }
+        client = self.make_client(("myremote", "mylab", "/root"), parent, files_by_folder)
+        attempted: list[str] = []
+
+        def download(file: File, path: str) -> bool:
+            attempted.append(file.name)
+            if file.name == "bad.txt":
+                raise OSError("Permission denied.")
+            return True
+
+        with tempfile.TemporaryDirectory() as tmp:
+            with (
+                patch("mdrsclient.transfer.FoldersApi") as folders_api_class,
+                patch("mdrsclient.transfer.FilesApi") as files_api_class,
+            ):
+                folders_api_class.return_value.retrieve.side_effect = lambda folder_id: folders[folder_id]
+                files_api_class.return_value.download.side_effect = download
+                with self.assertRaises(UnexpectedException):
+                    Downloader(client).download("myremote:/mylab/root", tmp, is_recursive=True)
+
+        self.assertEqual(sorted(attempted), ["bad.txt", "good1.txt", "good2.txt"])
+
+
+class TestRecursiveDownloadResilience(unittest.TestCase):
+    """A folder the client cannot prepare locally must not abandon its siblings."""
+
+    def test_a_folder_that_cannot_be_created_does_not_stop_the_walk(self):
+        parent = make_folder("p1", "lab", "/", [make_folder_simple("f1", "root")])
+        folders = {
+            "f1": make_folder("f1", "root", "/root/", [make_folder_simple("fa", "a"), make_folder_simple("fb", "b")]),
+            "fa": make_folder("fa", "a", "/root/a/"),
+            "fb": make_folder("fb", "b", "/root/b/"),
+        }
+        files_by_folder = {"p1": [], "f1": [], "fa": [make_file("good1", "good1.txt")], "fb": []}
+        client = MagicMock()
+        client.is_doi.return_value = False
+        client.parse_remote_host_with_path.return_value = ("myremote", "mylab", "/root")
+        client.find_laboratory.return_value = LABORATORY
+        client.find_folder.return_value = parent
+        client.find_files.side_effect = lambda folder_id: files_by_folder.get(folder_id, [])
+
+        real_makedirs = os.makedirs
+
+        def makedirs(path, *args, **kwargs):
+            if os.path.basename(path) == "b":
+                raise PermissionError("Permission denied")
+            return real_makedirs(path, *args, **kwargs)
+
+        attempted: list[str] = []
+
+        with tempfile.TemporaryDirectory() as tmp:
+            with (
+                patch("mdrsclient.transfer.FoldersApi") as folders_api_class,
+                patch("mdrsclient.transfer.FilesApi") as files_api_class,
+                patch("mdrsclient.transfer.os.makedirs", side_effect=makedirs),
+            ):
+                folders_api_class.return_value.retrieve.side_effect = lambda folder_id: folders[folder_id]
+                files_api_class.return_value.download.side_effect = lambda file, path: attempted.append(file.name)
+                with self.assertRaises(UnexpectedException):
+                    Downloader(client).download("myremote:/mylab/root", tmp, is_recursive=True)
+
+        self.assertEqual(attempted, ["good1.txt"])
+
+
+class TestFileDownloadPermission(unittest.TestCase):
+    """A file the client could not write is a failure, not a line of successful output."""
+
+    def make_connection(self, chunks) -> MagicMock:
+        connection = MagicMock()
+        connection.token = None
+        response = MagicMock()
+        response.status_code = 200
+        response.iter_content.return_value = chunks
+        connection.get.return_value = response
+        return connection
+
+    def test_a_download_that_cannot_be_written_is_reported(self):
+        connection = self.make_connection([b"payload"])
+        with tempfile.TemporaryDirectory() as tmp:
+            path = os.path.join(tmp, "precious.dat")
+            with open(path, "wb") as f:
+                f.write(b"do not touch")
+            os.chmod(path, 0o444)
+            try:
+                with open(path, "r+b"):
+                    self.skipTest("running with rights that ignore the file mode")
+            except OSError:
+                pass
+
+            with self.assertRaises(UnexpectedException):
+                FilesApi(connection).download(make_file("x1", "precious.dat"), path)
+
+            os.chmod(path, 0o644)
+            with open(path, "rb") as f:
+                self.assertEqual(f.read(), b"do not touch")
+            connection.get.assert_not_called()
+
+    def test_an_interrupted_download_leaves_the_existing_file_intact(self):
+        def chunks():
+            yield b"half a file"
+            raise ConnectionError("connection reset")
+
+        connection = self.make_connection(chunks())
+        with tempfile.TemporaryDirectory() as tmp:
+            path = os.path.join(tmp, "existing.dat")
+            with open(path, "wb") as f:
+                f.write(b"the copy already here")
+
+            with self.assertRaises(ConnectionError):
+                FilesApi(connection).download(make_file("x1", "existing.dat"), path)
+
+            with open(path, "rb") as f:
+                self.assertEqual(f.read(), b"the copy already here")
+            self.assertEqual(os.listdir(tmp), ["existing.dat"])
+
+    def test_a_finished_download_replaces_the_destination(self):
+        connection = self.make_connection([b"new ", b"contents"])
+        with tempfile.TemporaryDirectory() as tmp:
+            path = os.path.join(tmp, "existing.dat")
+            with open(path, "wb") as f:
+                f.write(b"old")
+
+            FilesApi(connection).download(make_file("x1", "existing.dat"), path)
+
+            with open(path, "rb") as f:
+                self.assertEqual(f.read(), b"new contents")
+            self.assertEqual(os.listdir(tmp), ["existing.dat"])
+
+
+if __name__ == "__main__":
+    unittest.main()