fix: Test connector error handling improvements

backend/connector_processor/connector_processor.py

@@ -12,8 +12,7 @@
     InValidConnectorId,
     InValidConnectorMode,
     OAuthTimeOut,
-    TestConnectorException,
-    TestConnectorInputException,
+    TestConnectorInputError,
 )
 
 from unstract.connectors.base import UnstractConnector
@@ -100,15 +99,15 @@ def get_all_supported_connectors(
         return supported_connectors
 
     @staticmethod
-    def test_connectors(connector_id: str, cred_string: dict[str, Any]) -> bool:
+    def test_connectors(connector_id: str, credentials: dict[str, Any]) -> bool:
         logger.info(f"Testing connector: {connector_id}")
         connector: dict[str, Any] = fetch_connectors_by_key_value(
             ConnectorKeys.ID, connector_id
         )[0]
         if connector.get(ConnectorKeys.OAUTH):
             try:
-                oauth_key = cred_string.get(ConnectorAuthKey.OAUTH_KEY)
-                cred_string = ConnectorAuthHelper.get_oauth_creds_from_cache(
+                oauth_key = credentials.get(ConnectorAuthKey.OAUTH_KEY)
+                credentials = ConnectorAuthHelper.get_oauth_creds_from_cache(
                     cache_key=oauth_key, delete_key=False
                 )
             except Exception as exc:
@@ -120,17 +119,13 @@ def test_connectors(connector_id: str, cred_string: dict[str, Any]) -> bool:
 
         try:
             connector_impl = Connectorkit().get_connector_by_id(
-                connector_id, cred_string
+                connector_id, credentials
             )
             test_result = connector_impl.test_credentials()
             logger.info(f"{connector_id} test result: {test_result}")
             return test_result
         except ConnectorError as e:
-            logger.error(f"Error while testing {connector_id}: {e}")
-            raise TestConnectorInputException(core_err=e)
-        except Exception as e:
-            logger.error(f"Error while testing {connector_id}: {e}")
-            raise TestConnectorException
+            raise TestConnectorInputError(core_err=e)
 
     def get_connector_data_with_key(connector_id: str, key_value: str) -> Any:
         """Generic Function to get connector data with provided key."""

backend/connector_processor/exceptions.py

@@ -31,7 +31,7 @@ class JSONParseException(APIException):
 
 class OAuthTimeOut(APIException):
     status_code = 408
-    default_detail = "Timed Out. Please re authenticate."
+    default_detail = "Timed out. Please re-authenticate."
 
 
 class InternalServiceError(APIException):
@@ -44,7 +44,7 @@ class TestConnectorException(APIException):
     default_detail = "Error while testing connector."
 
 
-class TestConnectorInputException(UnstractBaseException):
+class TestConnectorInputError(UnstractBaseException):
     def __init__(self, core_err: ConnectorError) -> None:
         super().__init__(detail=core_err.message, core_err=core_err)
         self.default_detail = core_err.message

backend/connector_processor/views.py

@@ -67,10 +67,10 @@ def test(self, request: Request) -> Response:
         """Tests the connector against the credentials passed."""
         serializer: TestConnectorSerializer = self.get_serializer(data=request.data)
         serializer.is_valid(raise_exception=True)
-        connector_id = serializer.validated_data.get(ConnectorKeys.CONNECTOR_ID)
+        connector_id = serializer.validated_data.get(CIKey.CONNECTOR_ID)
         cred_string = serializer.validated_data.get(CIKey.CONNECTOR_METADATA)
         test_result = ConnectorProcessor.test_connectors(
-            connector_id=connector_id, cred_string=cred_string
+            connector_id=connector_id, credentials=cred_string
         )
         return Response(
             {ConnectorKeys.IS_VALID: test_result},

unstract/connectors/pyproject.toml

@@ -16,7 +16,7 @@ dependencies = [
     "s3fs[boto3]==2023.6.0", # For Minio
     "PyDrive2[fsspec]==1.15.4", # For GDrive
     "oauth2client==4.1.3", # For GDrive
-    "dropboxdrivefs==1.3.1", # For Dropbox
+    "dropboxdrivefs==1.4.1", # For Dropbox
     "boxfs==0.2.1", # For Box
     "gcsfs==2023.6.0", # For GoogleCloudStorage
     "adlfs==2023.8.0", # For AzureCloudStorage

unstract/connectors/src/unstract/connectors/databases/unstract_db.py

@@ -68,7 +68,7 @@ def test_credentials(self) -> bool:
         try:
             self.get_engine()
         except Exception as e:
-            raise ConnectorError(str(e)) from e
+            raise ConnectorError(f"Error while connecting to DB: {str(e)}") from e
         return True
 
     def execute(self, query: str) -> Any:

unstract/connectors/src/unstract/connectors/filesystems/azure_cloud_storage/azure_cloud_storage.py

@@ -65,7 +65,11 @@ def get_fsspec_fs(self) -> AzureBlobFileSystem:
     def test_credentials(self) -> bool:
         """To test credentials for Azure Cloud Storage."""
         try:
-            self.get_fsspec_fs().ls(f"{self.bucket}")
+            is_dir = bool(self.get_fsspec_fs().isdir(self.bucket))
+            if not is_dir:
+                raise RuntimeError(f"'{self.bucket}' is not a valid bucket.")
         except Exception as e:
-            raise ConnectorError(str(e))
+            raise ConnectorError(
+                f"Error from Azure Cloud Storage while testing connection: {str(e)}"
+            ) from e
         return True

unstract/connectors/src/unstract/connectors/filesystems/box/box.py

@@ -24,7 +24,7 @@ def __init__(self, settings: dict[str, Any]):
                 settings_dict = json.loads(settings["box_app_settings"])
                 if not isinstance(settings_dict, dict):
                     raise ConnectorError(
-                        "Box app settings is expected to be a valid JSON",
+                        "Box app settings should be a valid JSON.",
                         treat_as_user_message=True,
                     )
             except JSONDecodeError as e:
@@ -112,8 +112,15 @@ def get_fsspec_fs(self) -> BoxFileSystem:
 
     def test_credentials(self) -> bool:
         """To test credentials for the Box connector."""
+        is_dir = False
         try:
-            self.get_fsspec_fs().isdir("/")
+            is_dir = bool(self.get_fsspec_fs().isdir("/"))
         except Exception as e:
-            raise ConnectorError(str(e))
+            raise ConnectorError(
+                f"Error from Box while testing connection: {str(e)}"
+            ) from e
+        if not is_dir:
+            raise ConnectorError(
+                "Unable to connect to Box, please check the connection settings."
+            )
         return True

unstract/connectors/src/unstract/connectors/filesystems/google_cloud_storage/google_cloud_storage.py

@@ -64,7 +64,11 @@ def get_fsspec_fs(self) -> GCSFileSystem:
     def test_credentials(self) -> bool:
         """To test credentials for Google Cloud Storage."""
         try:
-            is_dir = bool(self.get_fsspec_fs().isdir(f"{self.bucket}"))
-            return is_dir
+            is_dir = bool(self.get_fsspec_fs().isdir(self.bucket))
+            if not is_dir:
+                raise RuntimeError(f"'{self.bucket}' is not a valid bucket.")
         except Exception as e:
-            raise ConnectorError(str(e))
+            raise ConnectorError(
+                f"Error from Google Cloud Storage while testing connection: {str(e)}"
+            ) from e
+        return True

unstract/connectors/src/unstract/connectors/filesystems/google_drive/google_drive.py

@@ -91,10 +91,18 @@ def get_fsspec_fs(self) -> GDriveFileSystem:
 
     def test_credentials(self) -> bool:
         """To test credentials for Google Drive."""
+        is_dir = False
         try:
-            self.get_fsspec_fs().isdir("root")
+            is_dir = bool(self.get_fsspec_fs().isdir("root"))
         except Exception as e:
-            raise ConnectorError(str(e))
+            raise ConnectorError(
+                f"Error from Google Drive while testing connection: {str(e)}"
+            ) from e
+        if not is_dir:
+            raise ConnectorError(
+                "Unable to connect to Google Drive, "
+                "please check the connection settings."
+            )
         return True
 
     @staticmethod

unstract/connectors/src/unstract/connectors/filesystems/http/http.py

@@ -71,8 +71,16 @@ def get_fsspec_fs(self) -> HTTPFileSystem:
 
     def test_credentials(self) -> bool:
         """To test credentials for HTTP(S)."""
+        is_dir = False
         try:
-            self.get_fsspec_fs().isdir("/")
+            is_dir = bool(self.get_fsspec_fs().isdir("/"))
         except Exception as e:
-            raise ConnectorError(str(e))
+            raise ConnectorError(
+                f"Error while connecting to HTTP server: {str(e)}"
+            ) from e
+        if not is_dir:
+            raise ConnectorError(
+                "Unable to connect to HTTP server, "
+                "please check the connection settings."
+            )
         return True

unstract/connectors/src/unstract/connectors/filesystems/local_storage/local_storage.py

@@ -4,6 +4,7 @@
 
 from fsspec.implementations.local import LocalFileSystem
 
+from unstract.connectors.exceptions import ConnectorError
 from unstract.connectors.filesystems.unstract_file_system import UnstractFileSystem
 
 logger = logging.getLogger(__name__)
@@ -60,9 +61,16 @@ def get_fsspec_fs(self) -> Any:
 
     def test_credentials(self, *args, **kwargs) -> bool:  # type:ignore
         """To test credentials for LocalStorage."""
+        is_dir = False
         try:
-            self.get_fsspec_fs().isdir("/")
+            is_dir = bool(self.get_fsspec_fs().isdir("/"))
         except Exception as e:
-            logger.error(f"Test creds failed: {e}")
-            return False
+            raise ConnectorError(
+                f"Error while connecting to local storage: {str(e)}"
+            ) from e
+        if not is_dir:
+            raise ConnectorError(
+                "Unable to connect to local storage, "
+                "please check the connection settings."
+            )
         return True

unstract/connectors/src/unstract/connectors/filesystems/minio/exceptions.py

@@ -0,0 +1,26 @@
+from unstract.connectors.exceptions import ConnectorError
+
+S3FS_EXC_TO_UNSTRACT_EXC = {
+    "The AWS Access Key Id you provided does not exist in our records": (
+        "Invalid Key (Access Key ID) provided, please provide a valid one."
+    ),
+    "The request signature we calculated does not match the signature you provided": (
+        "Invalid Secret (Secret Access Key) provided, please provide a valid one."
+    ),
+    "[Errno 22] S3 API Requests must be made to API port": (  # Minio only
+        "Request made to invalid port, please check the port of the endpoint URL."
+    ),
+}
+
+
+def handle_s3fs_exception(e: Exception) -> ConnectorError:
+    original_exc = str(e)
+    user_msg = "Error from S3 / MinIO while testing connection: "
+    exc_to_append = ""
+    for s3fs_exc, user_friendly_msg in S3FS_EXC_TO_UNSTRACT_EXC.items():
+        if s3fs_exc in original_exc:
+            exc_to_append = user_friendly_msg
+            break
+
+    user_msg += exc_to_append if exc_to_append else str(e)
+    return ConnectorError(message=user_msg)

unstract/connectors/src/unstract/connectors/filesystems/minio/minio.py

@@ -4,9 +4,10 @@
 
 from s3fs.core import S3FileSystem
 
-from unstract.connectors.exceptions import ConnectorError
 from unstract.connectors.filesystems.unstract_file_system import UnstractFileSystem
 
+from .exceptions import handle_s3fs_exception
+
 logger = logging.getLogger(__name__)
 
 
@@ -37,11 +38,11 @@ def get_id() -> str:
 
     @staticmethod
     def get_name() -> str:
-        return "MinioFS/S3"
+        return "S3/Minio"
 
     @staticmethod
     def get_description() -> str:
-        return "All MinioFS compatible, including AWS S3"
+        return "Connect to AWS S3 and other compatible storage such as Minio."
 
     @staticmethod
     def get_icon() -> str:
@@ -76,7 +77,9 @@ def get_fsspec_fs(self) -> S3FileSystem:
     def test_credentials(self) -> bool:
         """To test credentials for Minio."""
         try:
-            self.get_fsspec_fs().isdir(f"{self.bucket}")
+            is_dir = bool(self.get_fsspec_fs().isdir(self.bucket))
+            if not is_dir:
+                raise RuntimeError(f"'{self.bucket}' is not a valid bucket.")
         except Exception as e:
-            raise ConnectorError(str(e))
+            raise handle_s3fs_exception(e) from e
         return True

unstract/connectors/src/unstract/connectors/filesystems/zs_dropbox/exceptions.py

@@ -7,16 +7,22 @@
 
 
 def handle_dropbox_exception(e: DropboxException) -> ConnectorError:
-    user_msg = ""
+    user_msg = "Error from Dropbox while testing connection: "
     if isinstance(e, ExcAuthError):
         if isinstance(e.error, AuthError):
             if e.error.is_expired_access_token():
-                user_msg = "Expired access token"
+                user_msg += (
+                    "Expired access token, please regenerate it "
+                    "through the Dropbox console."
+                )
             elif e.error.is_invalid_access_token():
-                user_msg = "Invalid access token"
+                user_msg += (
+                    "Invalid access token, please enter a valid token "
+                    "from the Dropbox console."
+                )
             else:
-                user_msg = e.error._tag
+                user_msg += e.error._tag
     elif isinstance(e, ApiError):
         if e.user_message_text is not None:
-            user_msg = e.user_message_text
+            user_msg += e.user_message_text
     return ConnectorError(message=user_msg, treat_as_user_message=True)

unstract/connectors/src/unstract/connectors/filesystems/zs_dropbox/zs_dropbox.py

@@ -69,11 +69,9 @@ def test_credentials(self) -> bool:
             # self.get_fsspec_fs().connect()
             self.get_fsspec_fs().ls("")
         except DropboxException as e:
-            logger.error(f"Test creds failed: {e}")
-            raise handle_dropbox_exception(e)
+            raise handle_dropbox_exception(e) from e
         except Exception as e:
-            logger.error(f"Test creds failed: {e}")
-            raise ConnectorError(str(e))
+            raise ConnectorError(f"Error while connecting to Dropbox: {str(e)}") from e
         return True
 
     @staticmethod