feat: Better error messages when config validation fails (#768)

samples/sample_tap_sqlite/__init__.py

@@ -48,6 +48,7 @@ class SQLiteTap(SQLTap):
             DB_PATH_CONFIG,
             th.StringType,
             description="The path to your SQLite database file(s).",
+            required=True,
             examples=["./path/to/my.db", "/absolute/path/to/my.db"],
         ),
     ).to_dict()

samples/sample_target_sqlite/__init__.py

@@ -52,6 +52,7 @@ class SQLiteTarget(SQLTarget):
             DB_PATH_CONFIG,
             th.StringType,
             description="The path to your SQLite database file(s).",
+            required=True,
         ),
     ).to_dict()
 

singer_sdk/exceptions.py

@@ -12,6 +12,21 @@
 class ConfigValidationError(Exception):
     """Raised when a user's config settings fail validation."""
 
+    def __init__(
+        self,
+        message: str,
+        *,
+        errors: list[str] | None = None,
+    ) -> None:
+        """Initialize a ConfigValidationError.
+
+        Args:
+            message: A message describing the error.
+            errors: A list of errors which caused the validation error.
+        """
+        super().__init__(message)
+        self.errors = errors or []
+
 
 class FatalAPIError(Exception):
     """Exception raised when a failed request should not be considered retriable."""

singer_sdk/plugin_base.py

@@ -72,6 +72,43 @@ def __init__(self) -> None:
         super().__init__("Mapper not initialized. Please call setup_mapper() first.")
 
 
+class SingerCommand(click.Command):
+    """Custom click command class for Singer packages."""
+
+    def __init__(
+        self,
+        *args: t.Any,
+        logger: logging.Logger,
+        **kwargs: t.Any,
+    ) -> None:
+        """Initialize the command.
+
+        Args:
+            *args: Positional `click.Command` arguments.
+            logger: A logger instance.
+            **kwargs: Keyword `click.Command` arguments.
+        """
+        super().__init__(*args, **kwargs)
+        self.logger = logger
+
+    def invoke(self, ctx: click.Context) -> t.Any:  # noqa: ANN401
+        """Invoke the command, capturing warnings and logging them.
+
+        Args:
+            ctx: The `click` context.
+
+        Returns:
+            The result of the command invocation.
+        """
+        logging.captureWarnings(capture=True)
+        try:
+            return super().invoke(ctx)
+        except ConfigValidationError as exc:
+            for error in exc.errors:
+                self.logger.error("Config validation error: %s", error)
+            sys.exit(1)
+
+
 class PluginBase(metaclass=abc.ABCMeta):
     """Abstract base class for taps."""
 
@@ -150,12 +187,12 @@ def __init__(
             if self._is_secret_config(k):
                 config_dict[k] = SecretString(v)
         self._config = config_dict
-        self._validate_config(raise_errors=validate_config)
-        self._mapper: PluginMapper | None = None
-
         metrics._setup_logging(self.config)
         self.metrics_logger = metrics.get_metrics_logger()
 
+        self._validate_config(raise_errors=validate_config)
+        self._mapper: PluginMapper | None = None
+
         # Initialization timestamp
         self.__initialized_at = int(time.time() * 1000)
 
@@ -351,27 +388,19 @@ def _is_secret_config(config_key: str) -> bool:
         """
         return is_common_secret_key(config_key)
 
-    def _validate_config(
-        self,
-        *,
-        raise_errors: bool = True,
-        warnings_as_errors: bool = False,
-    ) -> tuple[list[str], list[str]]:
+    def _validate_config(self, *, raise_errors: bool = True) -> list[str]:
         """Validate configuration input against the plugin configuration JSON schema.
 
         Args:
             raise_errors: Flag to throw an exception if any validation errors are found.
-            warnings_as_errors: Flag to throw an exception if any warnings were emitted.
 
         Returns:
-            A tuple of configuration validation warnings and errors.
+            A list of validation errors.
 
         Raises:
             ConfigValidationError: If raise_errors is True and validation fails.
         """
-        warnings: list[str] = []
         errors: list[str] = []
-        log_fn = self.logger.info
         config_jsonschema = self.config_jsonschema
 
         if config_jsonschema:
@@ -389,19 +418,11 @@ def _validate_config(
                 f"JSONSchema was: {config_jsonschema}"
             )
             if raise_errors:
-                raise ConfigValidationError(summary)
+                raise ConfigValidationError(summary, errors=errors)
 
-            log_fn = self.logger.warning
-        else:
-            summary = f"Config validation passed with {len(warnings)} warnings."
-            for warning in warnings:
-                summary += f"\n{warning}"
+            self.logger.warning(summary)
 
-        if warnings_as_errors and raise_errors and warnings:
-            msg = f"One or more warnings ocurred during validation: {warnings}"
-            raise ConfigValidationError(msg)
-        log_fn(summary)
-        return warnings, errors
+        return errors
 
     @classmethod
     def print_version(
@@ -555,7 +576,7 @@ def get_singer_command(cls: type[PluginBase]) -> click.Command:
         Returns:
             A callable CLI object.
         """
-        return click.Command(
+        return SingerCommand(
             name=cls.name,
             callback=cls.invoke,
             context_settings={"help_option_names": ["--help"]},
@@ -596,6 +617,7 @@ def get_singer_command(cls: type[PluginBase]) -> click.Command:
                     is_eager=True,
                 ),
             ],
+            logger=cls.logger,
         )
 
     @plugin_cli

tests/core/conftest.py

@@ -0,0 +1,101 @@
+"""Tap, target and stream test fixtures."""
+
+from __future__ import annotations
+
+import typing as t
+
+import pendulum
+import pytest
+
+from singer_sdk import Stream, Tap
+from singer_sdk.typing import (
+    DateTimeType,
+    IntegerType,
+    PropertiesList,
+    Property,
+    StringType,
+)
+
+
+class SimpleTestStream(Stream):
+    """Test stream class."""
+
+    name = "test"
+    schema = PropertiesList(
+        Property("id", IntegerType, required=True),
+        Property("value", StringType, required=True),
+        Property("updatedAt", DateTimeType, required=True),
+    ).to_dict()
+    replication_key = "updatedAt"
+
+    def __init__(self, tap: Tap):
+        """Create a new stream."""
+        super().__init__(tap, schema=self.schema, name=self.name)
+
+    def get_records(
+        self,
+        context: dict | None,  # noqa: ARG002
+    ) -> t.Iterable[dict[str, t.Any]]:
+        """Generate records."""
+        yield {"id": 1, "value": "Egypt"}
+        yield {"id": 2, "value": "Germany"}
+        yield {"id": 3, "value": "India"}
+
+
+class UnixTimestampIncrementalStream(SimpleTestStream):
+    name = "unix_ts"
+    schema = PropertiesList(
+        Property("id", IntegerType, required=True),
+        Property("value", StringType, required=True),
+        Property("updatedAt", IntegerType, required=True),
+    ).to_dict()
+    replication_key = "updatedAt"
+
+
+class UnixTimestampIncrementalStream2(UnixTimestampIncrementalStream):
+    name = "unix_ts_override"
+
+    def compare_start_date(self, value: str, start_date_value: str) -> str:
+        """Compare a value to a start date value."""
+
+        start_timestamp = pendulum.parse(start_date_value).format("X")
+        return max(value, start_timestamp, key=float)
+
+
+class SimpleTestTap(Tap):
+    """Test tap class."""
+
+    name = "test-tap"
+    config_jsonschema = PropertiesList(
+        Property("username", StringType, required=True),
+        Property("password", StringType, required=True),
+        Property("start_date", DateTimeType),
+        additional_properties=False,
+    ).to_dict()
+
+    def discover_streams(self) -> list[Stream]:
+        """List all streams."""
+        return [
+            SimpleTestStream(self),
+            UnixTimestampIncrementalStream(self),
+            UnixTimestampIncrementalStream2(self),
+        ]
+
+
+@pytest.fixture
+def tap_class():
+    """Return the tap class."""
+    return SimpleTestTap
+
+
+@pytest.fixture
+def tap() -> SimpleTestTap:
+    """Tap instance."""
+    return SimpleTestTap(
+        config={
+            "username": "utest",
+            "password": "ptest",
+            "start_date": "2021-01-01",
+        },
+        parse_env_config=False,
+    )

tests/core/test_mapper_class.py

@@ -0,0 +1,54 @@
+from __future__ import annotations
+
+import json
+from contextlib import nullcontext
+
+import pytest
+from click.testing import CliRunner
+
+from samples.sample_mapper.mapper import StreamTransform
+from singer_sdk.exceptions import ConfigValidationError
+
+
+@pytest.mark.parametrize(
+    "config_dict,expectation,errors",
+    [
+        pytest.param(
+            {},
+            pytest.raises(ConfigValidationError, match="Config validation failed"),
+            ["'stream_maps' is a required property"],
+            id="missing_stream_maps",
+        ),
+        pytest.param(
+            {"stream_maps": {}},
+            nullcontext(),
+            [],
+            id="valid_config",
+        ),
+    ],
+)
+def test_config_errors(config_dict: dict, expectation, errors: list[str]):
+    with expectation as exc:
+        StreamTransform(config=config_dict, validate_config=True)
+
+    if isinstance(exc, pytest.ExceptionInfo):
+        assert exc.value.errors == errors
+
+
+def test_cli_help():
+    """Test the CLI help message."""
+    runner = CliRunner(mix_stderr=False)
+    result = runner.invoke(StreamTransform.cli, ["--help"])
+    assert result.exit_code == 0
+    assert "Show this message and exit." in result.output
+
+
+def test_cli_config_validation(tmp_path):
+    """Test the CLI config validation."""
+    runner = CliRunner(mix_stderr=False)
+    config_path = tmp_path / "config.json"
+    config_path.write_text(json.dumps({}))
+    result = runner.invoke(StreamTransform.cli, ["--config", str(config_path)])
+    assert result.exit_code == 1
+    assert not result.stdout
+    assert "'stream_maps' is a required property" in result.stderr