ENH: Handle response errors in the file (hoechenberger#80)

Bad-response files can be silently downloaded, see OpenNeuroOrg/openneuro#2690. When doing: ``` $ python -m openneuro download --dataset=ds004107 --include="sub-mind002/ses-01/meg/sub-mind002_ses-01_task-auditory_events.tsv" --target_dir=~/mne_data/ds004107 ``` I get the TSV file with contents: ``` {"error": "an unknown error occurred accessing this file"} ``` So this PR: 1. Adds support for `python -m openneuro` by adding `__main__` 2. Adds support for `--target_dir=~/mne_data/ds004107` by adding `.expanduser().resolve()` on `target_dir` 3. Handles the case where the file contents are JSON-encoded `dict` with a single entry key `'error'`, which I think we can safely treat as an error It's possible that there is some response code earlier that we should be finding/detecting, but I'm not sure where that would be. Co-authored-by: Richard Höchenberger <richard.hoechenberger@gmail.com>
adam2392 · Oct 18, 2022 · 524329f · 524329f
1 parent 04364cd
commit 524329f
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 1 deletion.
diff --git a/.gitignore b/.gitignore
@@ -6,3 +6,4 @@ dist/
 build/
 .vscode/
 openneuro_py.egg-info/
+.hypothesis/
diff --git a/openneuro/__main__.py b/openneuro/__main__.py
@@ -0,0 +1,4 @@
+from .openneuro import cli
+
+if __name__ == '__main__':
+    cli()
diff --git a/openneuro/download.py b/openneuro/download.py
@@ -437,7 +437,12 @@ async def _retrieve_and_write_to_disk(
                     hash.update(chunk)
 
         if verify_hash and remote_file_hash is not None:
-            assert hash.hexdigest() == remote_file_hash
+            got = hash.hexdigest()
+            if got != remote_file_hash:
+                raise RuntimeError(
+                    f'Hash mismatch for:\n{outfile}\n'
+                    f'Expected:\n{remote_file_hash}\nGot:\n{got}'
+                )
 
         # Check the file was completely downloaded.
         if verify_size:
@@ -449,6 +454,23 @@ async def _retrieve_and_write_to_disk(
                     f'Server claimed size of {outfile} would be '
                     f'{remote_file_size} bytes, but downloaded '
                     f'{local_file_size} bytes.')
+    # Secondary check: try loading as JSON for "error" entry
+    # We can get for invalid files sometimes the contents:
+    # {"error": "an unknown error occurred accessing this file"}
+    # This is a 58-byte file, but let's be tolerant and try loading
+    # anything less than 200 as JSON and detect a dict with a single
+    # "error" entry.
+    if verify_size and local_file_size < 200:
+        try:
+            data = json.loads(outfile.read_text('utf-8'))
+        except Exception:
+            pass
+        else:
+            if isinstance(data, dict) and list(data) == ['error']:
+                raise RuntimeError(
+                    f'Error downloading:\n{outfile}:\n'
+                    f'Got JSON error response contents:\n{data}'
+                )
 
 
 async def _download_files(*,
@@ -685,6 +707,7 @@ def download(*,
         target_dir = Path(dataset)
     else:
         target_dir = Path(target_dir)
+    target_dir = target_dir.expanduser().resolve()
 
     include = [include] if isinstance(include, str) else include
     include = [] if include is None else list(include)