Show error message for access deny

Author: slinjhu

smoosense-gui/src/lib/features/folderTree/folderTreeSlice.ts

@@ -57,7 +57,17 @@ export const loadFolderContents = createAsyncThunk(
 
     const response = await fetch(`${API_PREFIX}/ls?${params}`)
     if (!response.ok) {
-      throw new Error(`Failed to load folder contents: ${response.statusText}`)
+      // Try to parse error message from API response
+      try {
+        const errorData = await response.json()
+        throw new Error(errorData.error || `Failed to load folder contents: ${response.statusText}`)
+      } catch (parseError) {
+        // If parsing fails, throw generic error
+        if (parseError instanceof Error && parseError.message !== `Failed to load folder contents: ${response.statusText}`) {
+          throw parseError
+        }
+        throw new Error(`Failed to load folder contents: ${response.statusText}`)
+      }
     }
 
     const items: FSItem[] = await response.json()

smoosense-py/smoosense/utils/s3_fs.py

@@ -6,6 +6,7 @@
 import boto3
 from pydantic import validate_call
 
+from smoosense.exceptions import AccessDeniedException
 from smoosense.utils.models import FSItem
 
 logger = logging.getLogger(__name__)
@@ -17,6 +18,8 @@ def __init__(self, s3_client: boto3.client):
 
     @validate_call()
     def list_one_level(self, key: str, limit: int = 100) -> list[FSItem]:
+        from botocore.exceptions import ClientError
+
         # Parse the S3 URL to get bucket and prefix
         parsed = urlparse(key)
         bucket = parsed.netloc
@@ -28,35 +31,43 @@ def list_one_level(self, key: str, limit: int = 100) -> list[FSItem]:
         paginator = self.s3_client.get_paginator("list_objects_v2")
         items = []
 
-        for page in paginator.paginate(Bucket=bucket, Prefix=prefix, Delimiter="/"):
-            # Add common prefixes (directories)
-            for prefix_entry in page.get("CommonPrefixes", []):
-                items.append(
-                    FSItem(
-                        name=os.path.basename(prefix_entry["Prefix"].rstrip("/")),
-                        size=0,
-                        lastModified=0,
-                        isDir=True,
+        try:
+            for page in paginator.paginate(Bucket=bucket, Prefix=prefix, Delimiter="/"):
+                # Add common prefixes (directories)
+                for prefix_entry in page.get("CommonPrefixes", []):
+                    items.append(
+                        FSItem(
+                            name=os.path.basename(prefix_entry["Prefix"].rstrip("/")),
+                            size=0,
+                            lastModified=0,
+                            isDir=True,
+                        )
                     )
-                )
-
-            # Add objects (files)
-            for obj in page.get("Contents", []):
-                # Skip the directory marker itself
-                if obj["Key"] == prefix:
-                    continue
-                items.append(
-                    FSItem(
-                        name=os.path.basename(obj["Key"]),
-                        size=obj["Size"],
-                        lastModified=int(obj["LastModified"].timestamp() * 1000),
-                        isDir=False,
+
+                # Add objects (files)
+                for obj in page.get("Contents", []):
+                    # Skip the directory marker itself
+                    if obj["Key"] == prefix:
+                        continue
+                    items.append(
+                        FSItem(
+                            name=os.path.basename(obj["Key"]),
+                            size=obj["Size"],
+                            lastModified=int(obj["LastModified"].timestamp() * 1000),
+                            isDir=False,
+                        )
                     )
-                )
 
-            if len(items) >= limit:
-                items = items[:limit]
-                break
+                if len(items) >= limit:
+                    items = items[:limit]
+                    break
+        except ClientError as e:
+            error_code = e.response.get("Error", {}).get("Code", "")
+            if error_code == "AccessDenied":
+                raise AccessDeniedException(str(e)) from e
+            if error_code == "NoSuchBucket":
+                raise FileNotFoundError(str(e)) from e
+            raise
 
         return items
 

smoosense-py/tests/test_s3_ls.py

@@ -0,0 +1,100 @@
+import json
+import unittest
+
+import boto3
+from flask import Flask
+
+from smoosense.handlers.fs import fs_bp
+from smoosense.my_logging import getLogger
+
+logger = getLogger(__name__)
+
+
+class TestS3LSEndpoint(unittest.TestCase):
+    """Test cases for the /ls endpoint with S3 paths."""
+
+    def setUp(self):
+        """Set up test fixtures before each test method."""
+        self.app = Flask(__name__)
+        self.app.register_blueprint(fs_bp)
+        self.app.config["TESTING"] = True
+        self.app.config["S3_CLIENT"] = boto3.client("s3")
+        self.client = self.app.test_client()
+
+        # Set up application context for all tests
+        self.app_context = self.app.app_context()
+        self.app_context.push()
+
+    def tearDown(self):
+        """Clean up after each test method."""
+        self.app_context.pop()
+
+    def test_ls_s3_bucket_root(self):
+        """Test listing S3 bucket root."""
+        response = self.client.get("/ls?path=s3://smoosense-demo/")
+        self.assertEqual(response.status_code, 200)
+
+        data = json.loads(response.get_data(as_text=True))
+        self.assertIsInstance(data, list)
+        self.assertGreater(len(data), 0)
+
+        # Each item should have required fields
+        for item in data:
+            self.assertIn("name", item)
+            self.assertIn("size", item)
+            self.assertIn("lastModified", item)
+            self.assertIn("isDir", item)
+
+    def test_ls_s3_nested_path(self):
+        """Test listing S3 nested path."""
+        response = self.client.get("/ls?path=s3://smoosense-demo/datasets/")
+        self.assertEqual(response.status_code, 200)
+
+        data = json.loads(response.get_data(as_text=True))
+        self.assertIsInstance(data, list)
+
+    def test_ls_s3_with_limit(self):
+        """Test listing S3 with limit parameter."""
+        response = self.client.get("/ls?path=s3://smoosense-demo/&limit=2")
+        self.assertEqual(response.status_code, 200)
+
+        data = json.loads(response.get_data(as_text=True))
+        self.assertIsInstance(data, list)
+        self.assertLessEqual(len(data), 2)
+
+    def test_ls_s3_nonexistent_bucket(self):
+        """Test listing non-existent S3 bucket returns 404."""
+        response = self.client.get("/ls?path=s3://this-bucket-definitely-does-not-exist-12345/")
+        self.assertEqual(response.status_code, 404)
+
+        data = json.loads(response.get_data(as_text=True))
+        self.assertIn("error", data)
+        self.assertIn("NoSuchBucket", data["error"])
+
+    def test_ls_s3_access_denied(self):
+        """Test listing S3 bucket without access returns 403."""
+        # Use a known bucket that exists but we don't have access to
+        response = self.client.get("/ls?path=s3://amazon-reviews-pds/")
+
+        # This should return 403 if access is denied
+        # Note: This test depends on not having access to this public bucket
+        # If you do have access, this test may pass with 200
+        if response.status_code == 403:
+            data = json.loads(response.get_data(as_text=True))
+            self.assertIn("error", data)
+            self.assertIn("AccessDenied", data["error"])
+        else:
+            # If we happen to have access, just verify it returns valid data
+            self.assertEqual(response.status_code, 200)
+
+    def test_ls_s3_missing_path(self):
+        """Test listing without path parameter returns 400."""
+        response = self.client.get("/ls")
+        self.assertEqual(response.status_code, 400)
+
+        data = json.loads(response.get_data(as_text=True))
+        self.assertIn("error", data)
+
+
+if __name__ == "__main__":
+    unittest.main()