diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 45f1ed55d..791906411 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -36,13 +36,44 @@ Unit tests do not require a running Langfuse server: uv run --frozen pytest -n auto --dist worksteal tests/unit ``` -E2E tests require a running Langfuse server and environment variables based on `.env.template`: +E2E tests require a running Langfuse server and environment variables based on `.env.template`. To start one locally with Docker (the same approach CI uses in `.github/workflows/ci.yml`): + +```bash +mkdir -p ./langfuse-server && cd ./langfuse-server +curl -fsSL https://raw.githubusercontent.com/langfuse/langfuse/main/docker-compose.yml -o docker-compose.yml + +# These auto-provision a project on first boot with keys matching .env.template, +# so no manual setup via the UI is needed. +LANGFUSE_INIT_ORG_ID=0c6c96f4-0ca0-4f16-92a8-6dd7d7c6a501 \ +LANGFUSE_INIT_ORG_NAME="SDK Test Org" \ +LANGFUSE_INIT_PROJECT_ID=7a88fb47-b4e2-43b8-a06c-a5ce950dc53a \ +LANGFUSE_INIT_PROJECT_NAME="SDK Test Project" \ +LANGFUSE_INIT_PROJECT_PUBLIC_KEY=pk-lf-1234567890 \ +LANGFUSE_INIT_PROJECT_SECRET_KEY=sk-lf-1234567890 \ +LANGFUSE_INIT_USER_EMAIL=sdk-tests@langfuse.local \ +LANGFUSE_INIT_USER_NAME="SDK Tests" \ +LANGFUSE_INIT_USER_PASSWORD=langfuse-ci-password \ +NEXT_PUBLIC_LANGFUSE_RUN_NEXT_INIT=true \ +docker compose up -d + +# Wait until this succeeds before running e2e tests; the stack takes a +# moment to become healthy on first boot. +curl --fail --retry 20 --retry-delay 5 --retry-connrefused http://localhost:3000/api/public/health +``` + +Then, back in the repo root (`cd ..` from `./langfuse-server`), with `.env` copied from `.env.template`: ```bash uv run --frozen pytest -n 4 --dist worksteal tests/e2e -m "not serial_e2e" uv run --frozen pytest tests/e2e -m "serial_e2e" ``` +To stop the local server when done: + +```bash +(cd ./langfuse-server && docker compose down -v) +``` + Live-provider tests make real provider calls and require provider API keys: ```bash diff --git a/README.md b/README.md index e21f83618..464debbb2 100644 --- a/README.md +++ b/README.md @@ -52,3 +52,7 @@ langfuse.flush() - Full documentation: https://langfuse.com/docs - Machine-readable docs index (for AI agents): https://langfuse.com/llms.txt - API reference of this package: https://python.reference.langfuse.com + +## Contributing + +See [CONTRIBUTING.md](CONTRIBUTING.md) for local setup, quality checks, and the test suite layout. diff --git a/langfuse/_client/client.py b/langfuse/_client/client.py index d3f699399..f3eacb4dd 100644 --- a/langfuse/_client/client.py +++ b/langfuse/_client/client.py @@ -377,8 +377,16 @@ def __init__( ) langfuse_logger.setLevel(logging.DEBUG) - public_key = public_key or os.environ.get(LANGFUSE_PUBLIC_KEY) - if public_key is None: + # Only fall back to the env var when the argument wasn't provided at + # all (None). An explicitly passed falsy value (e.g. public_key="") + # must be respected and rejected by the check below, not silently + # replaced by a real key that happens to be set in the environment. + public_key = ( + public_key + if public_key is not None + else os.environ.get(LANGFUSE_PUBLIC_KEY) + ) + if not isinstance(public_key, str) or not public_key.strip(): langfuse_logger.warning( "Authentication error: Langfuse client initialized without public_key. Client will be disabled. " "Provide a public_key parameter or set LANGFUSE_PUBLIC_KEY environment variable. " @@ -386,8 +394,12 @@ def __init__( self._otel_tracer = otel_trace_api.NoOpTracer() return - secret_key = secret_key or os.environ.get(LANGFUSE_SECRET_KEY) - if secret_key is None: + secret_key = ( + secret_key + if secret_key is not None + else os.environ.get(LANGFUSE_SECRET_KEY) + ) + if not isinstance(secret_key, str) or not secret_key.strip(): langfuse_logger.warning( "Authentication error: Langfuse client initialized without secret_key. Client will be disabled. " "Provide a secret_key parameter or set LANGFUSE_SECRET_KEY environment variable. " diff --git a/langfuse/media.py b/langfuse/media.py index 80e5bcd6a..e19560054 100644 --- a/langfuse/media.py +++ b/langfuse/media.py @@ -253,11 +253,19 @@ def parse_reference_string(reference_string: str) -> ParsedMediaReference: parsed_data = {} for pair in pairs: + if "=" not in pair: + continue key, value = pair.split("=", 1) parsed_data[key] = value - # Verify all required fields are present - if not all(key in parsed_data for key in ["type", "id", "source"]): + # Verify all required fields are present and non-blank. An empty or + # whitespace-only value (e.g. "id=" or "id= ") would otherwise pass + # this check and later reach a real API call + # (LangfuseMedia.resolve_media_references -> api.media.get) with a + # blank media_id. + if not all( + parsed_data.get(key, "").strip() for key in ["type", "id", "source"] + ): raise ValueError("Missing required fields in reference string") return ParsedMediaReference( diff --git a/tests/unit/test_initialization.py b/tests/unit/test_initialization.py index 6664d318f..9c06bdf22 100644 --- a/tests/unit/test_initialization.py +++ b/tests/unit/test_initialization.py @@ -7,6 +7,7 @@ import os import pytest +from opentelemetry.trace import NoOpTracer from langfuse import Langfuse from langfuse._client.resource_manager import LangfuseResourceManager @@ -297,3 +298,124 @@ def test_https_and_http_urls(self, cleanup_env_vars): secret_key="test_sk", ) assert client2._base_url == "http://insecure.com" + + +class TestCredentialValidation: + """An empty-string key (e.g. an unset secret rendered as "" by a + templated Docker/k8s env file) must disable the client the same way a + missing key does, rather than silently proceeding to construct a client + that then fails confusingly on every real request. + """ + + @pytest.fixture(autouse=True) + def cleanup(self): + original = { + "LANGFUSE_PUBLIC_KEY": os.environ.get("LANGFUSE_PUBLIC_KEY"), + "LANGFUSE_SECRET_KEY": os.environ.get("LANGFUSE_SECRET_KEY"), + } + + yield + + with LangfuseResourceManager._lock: + LangfuseResourceManager._instances.clear() + + for key, value in original.items(): + if value is None: + os.environ.pop(key, None) + else: + os.environ[key] = value + + def test_empty_string_public_key_env_var_disables_client(self, monkeypatch): + monkeypatch.setenv("LANGFUSE_PUBLIC_KEY", "") + monkeypatch.setenv("LANGFUSE_SECRET_KEY", "test_sk") + + client = Langfuse() + + assert isinstance(client._otel_tracer, NoOpTracer) + assert client._resources is None + + def test_empty_string_secret_key_env_var_disables_client(self, monkeypatch): + monkeypatch.setenv("LANGFUSE_PUBLIC_KEY", "test_pk") + monkeypatch.setenv("LANGFUSE_SECRET_KEY", "") + + client = Langfuse() + + assert isinstance(client._otel_tracer, NoOpTracer) + assert client._resources is None + + def test_empty_string_public_key_argument_disables_client(self, monkeypatch): + monkeypatch.delenv("LANGFUSE_PUBLIC_KEY", raising=False) + + client = Langfuse(public_key="", secret_key="test_sk") + + assert isinstance(client._otel_tracer, NoOpTracer) + assert client._resources is None + + def test_whitespace_only_public_key_argument_disables_client(self, monkeypatch): + # A whitespace-only value is falsy-string-adjacent but not caught by + # a plain truthiness check (" " is truthy); it must still disable + # the client rather than proceed with a blank credential. + monkeypatch.delenv("LANGFUSE_PUBLIC_KEY", raising=False) + + client = Langfuse(public_key=" ", secret_key="test_sk") + + assert isinstance(client._otel_tracer, NoOpTracer) + assert client._resources is None + + def test_whitespace_only_secret_key_argument_disables_client(self, monkeypatch): + monkeypatch.delenv("LANGFUSE_SECRET_KEY", raising=False) + + client = Langfuse(public_key="test_pk", secret_key=" ") + + assert isinstance(client._otel_tracer, NoOpTracer) + assert client._resources is None + + def test_non_string_public_key_argument_disables_client_without_crashing( + self, monkeypatch + ): + # A caller passing a non-string value (violating the declared + # Optional[str] type at runtime, since Python doesn't enforce it) + # must still disable the client cleanly rather than crash inside + # the validation guard itself (e.g. calling .strip() on an int). + monkeypatch.delenv("LANGFUSE_PUBLIC_KEY", raising=False) + + client = Langfuse(public_key=123, secret_key="test_sk") # type: ignore[arg-type] + + assert isinstance(client._otel_tracer, NoOpTracer) + assert client._resources is None + + def test_explicit_empty_public_key_argument_is_not_overridden_by_env_var( + self, monkeypatch + ): + # An explicitly passed empty public_key must be respected (and + # disable the client) rather than silently falling back to a real + # key that happens to be set in the environment -- e.g. a shared + # dev machine or CI runner with LANGFUSE_PUBLIC_KEY already exported. + monkeypatch.setenv("LANGFUSE_PUBLIC_KEY", "pk-real-env-key") + + client = Langfuse(public_key="", secret_key="test_sk") + + assert isinstance(client._otel_tracer, NoOpTracer) + assert client._resources is None + + def test_explicit_empty_secret_key_argument_is_not_overridden_by_env_var( + self, monkeypatch + ): + monkeypatch.setenv("LANGFUSE_SECRET_KEY", "sk-real-env-key") + + client = Langfuse(public_key="test_pk", secret_key="") + + assert isinstance(client._otel_tracer, NoOpTracer) + assert client._resources is None + + def test_omitted_public_key_still_falls_back_to_env_var(self, monkeypatch): + # Sanity check for the precedence fix: when the argument is truly + # omitted (not passed at all, so it defaults to None), the env var + # fallback must still work exactly as before. + monkeypatch.setenv("LANGFUSE_PUBLIC_KEY", "pk-from-env") + monkeypatch.setenv("LANGFUSE_SECRET_KEY", "sk-from-env") + + client = Langfuse() + + assert not isinstance(client._otel_tracer, NoOpTracer) + assert client._resources is not None diff --git a/tests/unit/test_media.py b/tests/unit/test_media.py index 343939722..551710686 100644 --- a/tests/unit/test_media.py +++ b/tests/unit/test_media.py @@ -91,6 +91,61 @@ def test_parse_invalid_reference_string(): ) # Missing fields +def test_parse_reference_string_with_empty_field_values_raises_missing_fields_error(): + # All three required keys present, but with empty values, used to pass + # the "key in parsed_data" check and return a reference with an empty + # media_id -- which would then reach a real api.media.get("") network + # call in LangfuseMedia.resolve_media_references. + with pytest.raises(ValueError, match="Missing required fields"): + LangfuseMedia.parse_reference_string("@@@langfuseMedia:type=|id=|source=@@@") + + +def test_parse_reference_string_with_whitespace_only_field_value_raises_missing_fields_error(): + # A whitespace-only value (e.g. "id= ") is non-empty as a raw string, + # so it would pass a plain truthiness check on the parsed value, and the + # resulting whitespace-only media_id would still reach a real + # api.media.get() call in LangfuseMedia.resolve_media_references. + with pytest.raises(ValueError, match="Missing required fields"): + LangfuseMedia.parse_reference_string( + "@@@langfuseMedia:type=image/jpeg|id= |source=bytes@@@" + ) + + +def test_parse_reference_string_with_malformed_pair_raises_missing_fields_error(): + # A pipe-separated segment with no "=" (here, a typo dropping the "=" + # from "id=") used to crash with a raw "not enough values to unpack" + # ValueError from the key/value split, instead of the intended + # "Missing required fields" validation error. + with pytest.raises(ValueError, match="Missing required fields"): + LangfuseMedia.parse_reference_string( + "@@@langfuseMedia:type=image/jpeg|idtest-id|source=bytes@@@" + ) + + +def test_parse_reference_string_ignores_malformed_pair_when_fields_still_present(): + # A malformed segment alongside all three required fields elsewhere in + # the string should be ignored, not crash the whole parse. + result = LangfuseMedia.parse_reference_string( + "@@@langfuseMedia:type=image/jpeg|badpair|id=test-id|source=bytes@@@" + ) + + assert result["media_id"] == "test-id" + assert result["content_type"] == "image/jpeg" + assert result["source"] == "bytes" + + +def test_parse_reference_string_ignores_trailing_empty_pair(): + # A trailing "|" produces an empty segment with no "=", which should be + # ignored rather than crash, as long as the required fields are present. + result = LangfuseMedia.parse_reference_string( + "@@@langfuseMedia:type=image/jpeg|id=test-id|source=bytes|@@@" + ) + + assert result["media_id"] == "test-id" + assert result["content_type"] == "image/jpeg" + assert result["source"] == "bytes" + + @pytest.mark.parametrize( ("url_expiry", "expected"), [ diff --git a/tests/unit/test_request_client.py b/tests/unit/test_request_client.py new file mode 100644 index 000000000..ba0bd23a1 --- /dev/null +++ b/tests/unit/test_request_client.py @@ -0,0 +1,135 @@ +"""Unit tests for LangfuseClient (langfuse/_utils/request.py) against a real, +local HTTP server. + +This exercises the low-level HTTP layer's behavior under real failure modes +that mocked-response tests do not catch: a malformed/non-JSON response body, +an error status code with a body Langfuse doesn't recognize, and a slow/hung +dependency that must be bounded by the client's configured timeout rather +than hanging indefinitely. +""" + +import time + +import httpx +import pytest +from werkzeug.wrappers import Response as WerkzeugResponse + +from langfuse._utils.request import APIError, APIErrors, LangfuseClient + + +@pytest.fixture +def client(httpserver): + return LangfuseClient( + public_key="test-public-key", + secret_key="test-secret-key", + base_url=httpserver.url_for("/"), + version="test-version", + timeout=5, + session=httpx.Client(), + ) + + +def test_batch_post_success(client, httpserver): + httpserver.expect_request("/api/public/ingestion").respond_with_json( + {"successes": [], "errors": []} + ) + + response = client.batch_post(batch=[]) + + assert response.status_code == 200 + + +def test_batch_post_malformed_json_response_raises_api_error(client, httpserver): + # A 200 response whose body isn't valid JSON must not crash with a raw + # json.JSONDecodeError; it should surface as the SDK's own APIError. + httpserver.expect_request("/api/public/ingestion").respond_with_data( + "not valid json", status=200, content_type="application/json" + ) + + with pytest.raises(APIError, match="Invalid JSON response received"): + client._process_response( + httpx.get(httpserver.url_for("/api/public/ingestion")), + success_message="ok", + return_json=True, + ) + + +def test_batch_post_207_with_errors_raises_api_errors(client, httpserver): + httpserver.expect_request("/api/public/ingestion").respond_with_json( + { + "errors": [ + {"status": 400, "message": "Bad request", "error": "Invalid event"} + ] + }, + status=207, + ) + + with pytest.raises(APIErrors): + client.batch_post(batch=[{"bad": "event"}]) + + +def test_batch_post_207_malformed_json_raises_api_error(client, httpserver): + httpserver.expect_request("/api/public/ingestion").respond_with_data( + "not valid json", status=207, content_type="application/json" + ) + + with pytest.raises(APIError, match="Invalid JSON response received"): + client.batch_post(batch=[{"some": "event"}]) + + +def test_batch_post_unauthorized_raises_api_error_with_status(client, httpserver): + httpserver.expect_request("/api/public/ingestion").respond_with_json( + {"message": "Unauthorized"}, status=401 + ) + + with pytest.raises(APIError) as exc_info: + client.batch_post(batch=[{"some": "event"}]) + + assert exc_info.value.status == 401 + + +def test_batch_post_generic_error_with_non_json_body_raises_api_error( + client, httpserver +): + # A 500 with a plain-text (non-JSON) body must not crash trying to parse + # it as JSON; it should fall back to the raw response text. + httpserver.expect_request("/api/public/ingestion").respond_with_data( + "Internal Server Error", status=500, content_type="text/plain" + ) + + with pytest.raises(APIError) as exc_info: + client.batch_post(batch=[{"some": "event"}]) + + assert exc_info.value.status == 500 + assert "Internal Server Error" in str(exc_info.value) + + +def test_batch_post_times_out_on_slow_server_instead_of_hanging(httpserver): + # A dependency that never responds must not hang the caller forever: the + # configured timeout has to actually be enforced end-to-end, not just + # accepted as a constructor argument. + def slow_handler(_request): + time.sleep(2) + return WerkzeugResponse("late", status=200) + + httpserver.expect_request("/api/public/ingestion").respond_with_handler( + slow_handler + ) + + slow_client = LangfuseClient( + public_key="test-public-key", + secret_key="test-secret-key", + base_url=httpserver.url_for("/"), + version="test-version", + timeout=0.2, + session=httpx.Client(), + ) + + start = time.monotonic() + with pytest.raises(httpx.TimeoutException): + slow_client.batch_post(batch=[{"some": "event"}]) + elapsed = time.monotonic() - start + + # Bounded well below the handler's 2s sleep: the client's own timeout + # fired rather than eventually receiving the late response. + assert elapsed < 1.0