fix: fs_s3.read() exceptions are not caught properly (#432)

* fix: fs_s3.read() exceptions are not caught properly

* fix: exceptions are catch in less specific to specific order

scripts/files/fs_s3.py

@@ -39,11 +39,14 @@ def read(path: str, needs_credentials: bool = False) -> bytes:
     """Read a file on a AWS S3 bucket.
 
     Args:
-        path (str): The AWS S3 path to the file to read.
-        needs_credentials (bool, optional):  Tells if credentials are needed. Defaults to False.
+        path: The AWS S3 path to the file to read.
+        needs_credentials:  Tells if credentials are needed. Defaults to False.
+
+    Raises:
+        ce: botocore ClientError
 
     Returns:
-        bytes: The file in bytes.
+        The file in bytes.
     """
     start_time = time_in_ms()
     s3_path = parse_path(path)
@@ -56,17 +59,18 @@ def read(path: str, needs_credentials: bool = False) -> bytes:
 
         s3_object = s3.Object(s3_path.bucket, key)
         file: bytes = s3_object.get()["Body"].read()
+    except s3.meta.client.exceptions.NoSuchBucket as nsb:
+        get_log().error("s3_bucket_not_found", path=path, error=f"The specified bucket does not seem to exist: {nsb}")
+        raise nsb
+    except s3.meta.client.exceptions.NoSuchKey as nsk:
+        get_log().error("s3_key_not_found", path=path, error=f"The specified file does not seem to exist: {nsk}")
+        raise nsk
     except s3.meta.client.exceptions.ClientError as ce:
+        # https://boto3.amazonaws.com/v1/documentation/api/latest/guide/error-handling.html#parsing-error-responses-and-catching-exceptions-from-aws-services
         if not needs_credentials and ce.response["Error"]["Code"] == "AccessDenied":
             get_log().debug("read_s3_needs_credentials", path=path)
             return read(path, True)
         raise ce
-    except s3.meta.client.exceptions.NoSuchBucket as nsb:
-        get_log().error("read_s3_bucket_not_found", path=path, error=f"The specified bucket does not seem to exist: {nsb}")
-        raise nsb
-    except s3.meta.client.exceptions.NoSuchKey as nsk:
-        get_log().error("read_s3_file_not_found", path=path, error=f"The specified file does not seem to exist: {nsk}")
-        raise nsk
 
     get_log().debug("read_s3_success", path=path, duration=time_in_ms() - start_time)
     return file

scripts/files/tests/fs_s3_test.py

@@ -1,3 +1,5 @@
+import json
+
 import boto3
 import botocore
 import pytest
@@ -36,16 +38,19 @@ def test_read() -> None:
 def test_read_bucket_not_found(capsys: CaptureFixture[str]) -> None:
     with pytest.raises(botocore.exceptions.ClientError):
         read("s3://testbucket/test.file")
-        sysout = capsys.readouterr()
-        assert "read_s3_bucket_not_found" in sysout.out
+
+    # python-linz-logger uses structlog which doesn't use stdlib so can't capture the logs with `caplog`
+    logs = json.loads(capsys.readouterr().out)
+    assert logs["msg"] == "s3_bucket_not_found"
 
 
 @mock_s3  # type: ignore
-def test_read_file_not_found(capsys: CaptureFixture[str]) -> None:
+def test_read_key_not_found(capsys: CaptureFixture[str]) -> None:
     s3 = boto3.resource("s3", region_name=DEFAULT_REGION_NAME)
     s3.create_bucket(Bucket="testbucket")
 
     with pytest.raises(botocore.exceptions.ClientError):
         read("s3://testbucket/test.file")
-        sysout = capsys.readouterr()
-        assert "read_s3_file_not_found" in sysout.out
+
+    logs = json.loads(capsys.readouterr().out)
+    assert logs["msg"] == "s3_key_not_found"