mirror of
https://github.com/Kaelio/ktx.git
synced 2026-07-04 10:52:13 +02:00
fix(telemetry): classify daemon query rejections as expected, not faults (#335)
* fix(telemetry): classify daemon query rejections as expected, not faults Semantic-layer query rejections and warehouse-execution rejections from the sl_query MCP tool were wrapped as generic Errors, so reportException filed them as PostHog $exception faults indistinguishable from real ktx bugs. The daemon already separates a caller rejection (planner ValueError -> exit 3 / HTTP 400) from a crash. The Node runner now carries that distinction as a typed KtxDaemonComputeError, and a shared throwClassifiedQueryError promotes daemon input-rejections and warehouse rejections to KtxQueryError while daemon crashes and native JS faults still reach Error Tracking. query_semantic_layer stops report_exception-ing expected ValueErrors, and a missing 'file:' secret now raises KtxExpectedError so absent .ktx/secrets/<conn>-password stops filing faults. * chore: sync uv.lock to ktx-daemon/ktx-sl 0.15.0
This commit is contained in:
parent
a651b82e2f
commit
5d17469601
10 changed files with 347 additions and 28 deletions
|
|
@ -35,6 +35,12 @@ from ktx_daemon.source_generation import (
|
|||
generate_sources_response,
|
||||
)
|
||||
|
||||
# The caller (the Node client) sent a well-formed request the compute layer
|
||||
# refused as invalid — e.g. the planner rejecting an agent's query. A distinct
|
||||
# exit code lets the client classify it as an expected outcome rather than a
|
||||
# ktx fault. Kept in sync with DAEMON_INPUT_REJECTED_EXIT_CODE on the Node side.
|
||||
INPUT_REJECTED_EXIT_CODE = 3
|
||||
|
||||
|
||||
def build_parser() -> argparse.ArgumentParser:
|
||||
parser = argparse.ArgumentParser(prog="ktx-daemon")
|
||||
|
|
@ -210,9 +216,17 @@ def main(argv: list[str] | None = None) -> int:
|
|||
return 2
|
||||
sys.stdout.write(response.model_dump_json() + "\n")
|
||||
return 0
|
||||
except (json.JSONDecodeError, ValidationError, ValueError) as error:
|
||||
except (json.JSONDecodeError, ValidationError) as error:
|
||||
# Malformed request envelope — ktx sent bad JSON or a mis-shaped payload.
|
||||
# That is a ktx fault, so keep the generic non-zero code (JSONDecodeError
|
||||
# subclasses ValueError, so this clause must precede the ValueError one).
|
||||
sys.stderr.write(f"{error}\n")
|
||||
return 1
|
||||
except ValueError as error:
|
||||
# The compute layer refused a well-formed request as invalid (e.g. the
|
||||
# planner rejecting the agent's measures). Expected, not a fault.
|
||||
sys.stderr.write(f"{error}\n")
|
||||
return INPUT_REJECTED_EXIT_CODE
|
||||
except Exception as error:
|
||||
from ktx_daemon.telemetry import report_exception
|
||||
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@ import time
|
|||
from typing import Any
|
||||
|
||||
from ktx_daemon.telemetry import error_class, report_exception, track_telemetry_event
|
||||
from pydantic import BaseModel, ConfigDict, Field
|
||||
from pydantic import BaseModel, ConfigDict, Field, ValidationError
|
||||
from semantic_layer.duplicate_check import validate_measure_duplicates
|
||||
from semantic_layer.engine import SemanticEngine
|
||||
from semantic_layer.models import QueryResult, SourceDefinition
|
||||
|
|
@ -150,13 +150,18 @@ def query_semantic_layer(
|
|||
track_telemetry_event(
|
||||
"sql_gen_completed", sql_fields, project_id=request.project_id
|
||||
)
|
||||
report_exception(
|
||||
error,
|
||||
source="semantic-query",
|
||||
handled=True,
|
||||
fatal=False,
|
||||
project_id=request.project_id,
|
||||
)
|
||||
# A ValueError is the engine refusing the caller's query — an expected
|
||||
# rejection surfaced to the agent, not a ktx fault. Keep it out of Error
|
||||
# Tracking (a pydantic ValidationError subclasses ValueError but means a
|
||||
# malformed request envelope, so it stays a reported fault).
|
||||
if not isinstance(error, ValueError) or isinstance(error, ValidationError):
|
||||
report_exception(
|
||||
error,
|
||||
source="semantic-query",
|
||||
handled=True,
|
||||
fatal=False,
|
||||
project_id=request.project_id,
|
||||
)
|
||||
raise
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -91,6 +91,22 @@ def test_command_returns_nonzero_for_invalid_json() -> None:
|
|||
assert "Expecting property name enclosed in double quotes" in result.stderr
|
||||
|
||||
|
||||
def test_semantic_query_rejects_invalid_query_with_input_rejected_code() -> None:
|
||||
# A planner ValueError (agent's measure references no source) is an expected
|
||||
# input rejection, distinguished from a fault by INPUT_REJECTED_EXIT_CODE (3).
|
||||
result = run_daemon_command(
|
||||
"semantic-query",
|
||||
{
|
||||
"sources": [ORDERS_SOURCE],
|
||||
"dialect": "postgres",
|
||||
"query": {"measures": ["count(*)"], "dimensions": []},
|
||||
},
|
||||
)
|
||||
|
||||
assert result.returncode == 3, result.stderr
|
||||
assert "does not reference any source" in result.stderr
|
||||
|
||||
|
||||
def test_serve_http_command_starts_uvicorn_without_reading_stdin(
|
||||
monkeypatch,
|
||||
) -> None:
|
||||
|
|
|
|||
|
|
@ -125,7 +125,7 @@ def test_query_semantic_layer_emits_plan_and_sql_debug_events(
|
|||
assert "public.orders" not in captured.err
|
||||
|
||||
|
||||
def test_query_semantic_layer_reports_exception(monkeypatch) -> None:
|
||||
def test_query_semantic_layer_reports_unexpected_fault(monkeypatch) -> None:
|
||||
from ktx_daemon import semantic_layer as semantic_layer_module
|
||||
|
||||
reports: list[dict[str, object]] = []
|
||||
|
|
@ -133,12 +133,16 @@ def test_query_semantic_layer_reports_exception(monkeypatch) -> None:
|
|||
def fake_report(exception: BaseException, **kwargs: object) -> None:
|
||||
reports.append({"exception": exception, **kwargs})
|
||||
|
||||
monkeypatch.setattr(semantic_layer_module, "report_exception", fake_report)
|
||||
def boom(*_args: object, **_kwargs: object) -> None:
|
||||
raise RuntimeError("engine construction failed")
|
||||
|
||||
with pytest.raises(ValueError):
|
||||
monkeypatch.setattr(semantic_layer_module, "report_exception", fake_report)
|
||||
monkeypatch.setattr(semantic_layer_module.SemanticEngine, "from_sources", boom)
|
||||
|
||||
with pytest.raises(RuntimeError):
|
||||
query_semantic_layer(
|
||||
SemanticLayerQueryRequest(
|
||||
sources=[ORDERS_SOURCE, ORDERS_SOURCE],
|
||||
sources=[ORDERS_SOURCE],
|
||||
dialect="postgres",
|
||||
projectId="a" * 64,
|
||||
query={"measures": ["orders.order_count"]},
|
||||
|
|
@ -152,6 +156,32 @@ def test_query_semantic_layer_reports_exception(monkeypatch) -> None:
|
|||
assert reports[0]["project_id"] == "a" * 64
|
||||
|
||||
|
||||
def test_query_semantic_layer_does_not_report_expected_query_rejection(
|
||||
monkeypatch,
|
||||
) -> None:
|
||||
from ktx_daemon import semantic_layer as semantic_layer_module
|
||||
|
||||
reports: list[dict[str, object]] = []
|
||||
monkeypatch.setattr(
|
||||
semantic_layer_module,
|
||||
"report_exception",
|
||||
lambda *_args, **kwargs: reports.append(kwargs),
|
||||
)
|
||||
|
||||
# A planner ValueError is the engine refusing the agent's query — surfaced to
|
||||
# the caller and re-raised, but never filed as a ktx fault.
|
||||
with pytest.raises(ValueError, match="does not reference any source"):
|
||||
query_semantic_layer(
|
||||
SemanticLayerQueryRequest(
|
||||
sources=[ORDERS_SOURCE],
|
||||
dialect="postgres",
|
||||
query={"measures": ["count(*)"]},
|
||||
)
|
||||
)
|
||||
|
||||
assert reports == []
|
||||
|
||||
|
||||
def test_semantic_layer_request_rejects_project_id_field_name() -> None:
|
||||
with pytest.raises(ValueError):
|
||||
SemanticLayerQueryRequest(
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue