-
-
Notifications
You must be signed in to change notification settings - Fork 22
feat(logging): server-side client log + crash capture (#106) #1436
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| import pytest | ||
| import pytest_asyncio | ||
|
|
||
| from tinyagentos.client_log_store import ClientLogStore | ||
|
|
||
|
|
||
| @pytest_asyncio.fixture | ||
| async def store(tmp_path): | ||
| s = ClientLogStore(tmp_path / "client_logs.db") | ||
| await s.init() | ||
| yield s | ||
| await s.close() | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_create_and_list(store): | ||
| await store.create( | ||
| user_id="u1", level="error", message="boom", | ||
| source="MessagesApp", url="/desktop", stack="at foo", | ||
| ) | ||
| items = await store.list_recent() | ||
| assert len(items) == 1 | ||
| assert items[0]["level"] == "error" | ||
| assert items[0]["message"] == "boom" | ||
| assert items[0]["source"] == "MessagesApp" | ||
| assert items[0]["url"] == "/desktop" | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_list_filters_by_level(store): | ||
| await store.create(user_id="u1", level="error", message="e") | ||
| await store.create(user_id="u1", level="info", message="i") | ||
| errs = await store.list_recent(level="error") | ||
| assert len(errs) == 1 | ||
| assert errs[0]["level"] == "error" | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_long_message_and_stack_are_truncated(store): | ||
| await store.create( | ||
| user_id="u1", level="error", | ||
| message="x" * 5000, stack="y" * 20000, | ||
| ) | ||
| item = (await store.list_recent())[0] | ||
| assert len(item["message"]) == 4000 | ||
| assert len(item["stack"]) == 16000 | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_ring_buffer_caps_total_rows(store, monkeypatch): | ||
| monkeypatch.setattr("tinyagentos.client_log_store.MAX_ROWS", 3) | ||
| for i in range(6): | ||
| await store.create(user_id="u1", level="debug", message=f"m{i}") | ||
| items = await store.list_recent(limit=100) | ||
| assert len(items) == 3 | ||
| # The newest entry is retained; the oldest are pruned. | ||
| msgs = [i["message"] for i in items] | ||
| assert "m5" in msgs | ||
| assert "m0" not in msgs | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| """Route tests for the client-log capture API.""" | ||
| import pytest | ||
|
|
||
| from tinyagentos.auth_context import CurrentUser, current_user | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SUGGESTION: There is no end-to-end test verifying that the Reply with |
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_post_client_log_returns_201(client): | ||
| resp = await client.post( | ||
| "/api/client-logs", | ||
| json={"level": "error", "message": "boom", "source": "MessagesApp", | ||
| "url": "/desktop", "stack": "at render"}, | ||
| ) | ||
| assert resp.status_code == 201 | ||
| d = resp.json() | ||
| assert d["level"] == "error" | ||
| assert d["message"] == "boom" | ||
| assert "id" in d and "created_at" in d | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_get_lists_posted_log(client): | ||
| await client.post("/api/client-logs", json={"level": "warn", "message": "hmm"}) | ||
| resp = await client.get("/api/client-logs") | ||
| assert resp.status_code == 200 | ||
| msgs = [i["message"] for i in resp.json()["items"]] | ||
| assert "hmm" in msgs | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_get_filters_by_level(client): | ||
| await client.post("/api/client-logs", json={"level": "error", "message": "E"}) | ||
| await client.post("/api/client-logs", json={"level": "info", "message": "I"}) | ||
| resp = await client.get("/api/client-logs?level=error") | ||
| items = resp.json()["items"] | ||
| assert items and all(i["level"] == "error" for i in items) | ||
| assert "E" in [i["message"] for i in items] | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_invalid_level_rejected(client): | ||
| resp = await client.post("/api/client-logs", json={"level": "bogus", "message": "x"}) | ||
| assert resp.status_code == 400 | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_empty_message_rejected(client): | ||
| resp = await client.post("/api/client-logs", json={"level": "error", "message": " "}) | ||
| assert resp.status_code == 400 | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_post_is_rate_limited_per_user(client, monkeypatch): | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SUGGESTION: The test only exercises a single user, so it does not verify that buckets are independent per user. The whole point of Reply with |
||
| from tinyagentos.rate_limit import RateLimiter | ||
|
|
||
| # Small, non-refilling bucket so a flood is deterministically capped: the | ||
| # 4th valid post past a capacity of 3 is rejected, protecting other users' | ||
| # entries in the shared ring buffer. | ||
| monkeypatch.setattr( | ||
| "tinyagentos.routes.client_logs._post_limiter", | ||
| RateLimiter(capacity=3, refill_per_second=0.0), | ||
| ) | ||
| for _ in range(3): | ||
| ok = await client.post("/api/client-logs", json={"level": "error", "message": "flood"}) | ||
| assert ok.status_code == 201 | ||
| blocked = await client.post("/api/client-logs", json={"level": "error", "message": "flood"}) | ||
| assert blocked.status_code == 429 | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_non_admin_can_post_but_not_read(app, client): | ||
| app.dependency_overrides[current_user] = lambda: CurrentUser(user_id="bob", is_admin=False) | ||
| try: | ||
| post = await client.post("/api/client-logs", json={"level": "error", "message": "bob-err"}) | ||
| assert post.status_code == 201 | ||
| resp = await client.get("/api/client-logs") | ||
| assert resp.status_code == 403 | ||
| finally: | ||
| app.dependency_overrides.pop(current_user, None) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,123 @@ | ||
| """Server-side capture of client (browser/PWA) logs and crashes. | ||
|
|
||
| In a PWA there is no devtools console for the user (or us) to read, so a | ||
| front-end crash is invisible unless the client ships it somewhere. This store is | ||
| that sink: the desktop posts errors, warnings, and debug lines to | ||
| POST /api/client-logs and they land here, readable by an admin via | ||
| GET /api/client-logs. It is the substrate for chasing crashes like the Messages | ||
| app failure (#106 log capture). | ||
|
|
||
| Bounded by design: a crash loop must not grow the table without limit, so every | ||
| insert prunes to the most recent MAX_ROWS rows (a ring buffer), and message/stack | ||
| text is length-capped. | ||
| """ | ||
| from __future__ import annotations | ||
|
|
||
| import uuid | ||
| from datetime import datetime, timezone | ||
|
|
||
| from tinyagentos.base_store import BaseStore | ||
|
|
||
| # The levels a client may report. Mirrors console severities plus an explicit | ||
| # "fatal" for an uncaught error / error-boundary crash. | ||
| VALID_LEVELS = frozenset({"fatal", "error", "warn", "info", "debug"}) | ||
|
|
||
| MAX_MESSAGE_LEN = 4_000 | ||
| MAX_STACK_LEN = 16_000 | ||
| MAX_SOURCE_LEN = 200 | ||
| MAX_URL_LEN = 1_000 | ||
| MAX_UA_LEN = 500 | ||
| # Ring-buffer cap: keep only the most recent N entries across all users so a | ||
| # crash loop posting on every render cannot grow the DB unbounded. | ||
| MAX_ROWS = 2_000 | ||
|
|
||
|
|
||
| class ClientLogStore(BaseStore): | ||
| SCHEMA = """ | ||
| CREATE TABLE IF NOT EXISTS client_logs ( | ||
| id TEXT NOT NULL PRIMARY KEY, | ||
| user_id TEXT NOT NULL, | ||
| level TEXT NOT NULL, | ||
| message TEXT NOT NULL, | ||
| source TEXT NOT NULL DEFAULT '', | ||
| url TEXT NOT NULL DEFAULT '', | ||
| stack TEXT NOT NULL DEFAULT '', | ||
| user_agent TEXT NOT NULL DEFAULT '', | ||
| created_at TEXT NOT NULL | ||
| ); | ||
| CREATE INDEX IF NOT EXISTS client_logs_created | ||
| ON client_logs (created_at DESC); | ||
| CREATE INDEX IF NOT EXISTS client_logs_level_created | ||
| ON client_logs (level, created_at DESC); | ||
| """ | ||
|
|
||
| async def create( | ||
| self, | ||
| *, | ||
| user_id: str, | ||
| level: str, | ||
| message: str, | ||
| source: str = "", | ||
| url: str = "", | ||
| stack: str = "", | ||
| user_agent: str = "", | ||
| ) -> dict: | ||
| assert self._db is not None | ||
| item_id = str(uuid.uuid4()) | ||
| created_at = datetime.now(timezone.utc).isoformat() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Edge Case: Ring-buffer prune/ordering relies on created_at with no tie-breaker
Suggested fix: add the primary key as a secondary sort key (e.g. Was this helpful? React with 👍 / 👎 |
||
| row = { | ||
| "id": item_id, | ||
| "user_id": user_id, | ||
| "level": level, | ||
| "message": message[:MAX_MESSAGE_LEN], | ||
| "source": source[:MAX_SOURCE_LEN], | ||
| "url": url[:MAX_URL_LEN], | ||
| "stack": stack[:MAX_STACK_LEN], | ||
| "user_agent": user_agent[:MAX_UA_LEN], | ||
| "created_at": created_at, | ||
| } | ||
| await self._db.execute( | ||
| """ | ||
| INSERT INTO client_logs | ||
| (id, user_id, level, message, source, url, stack, user_agent, created_at) | ||
| VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) | ||
| """, | ||
| ( | ||
| row["id"], row["user_id"], row["level"], row["message"], | ||
| row["source"], row["url"], row["stack"], row["user_agent"], | ||
| row["created_at"], | ||
| ), | ||
| ) | ||
| # Ring-buffer prune: drop everything older than the newest MAX_ROWS. | ||
| await self._db.execute( | ||
| """ | ||
| DELETE FROM client_logs WHERE id NOT IN ( | ||
| SELECT id FROM client_logs ORDER BY created_at DESC LIMIT ? | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WARNING: Ring-buffer prune Reply with |
||
| ) | ||
| """, | ||
| (MAX_ROWS,), | ||
| ) | ||
| await self._db.commit() | ||
|
Comment on lines
+91
to
+100
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Performance: Ring-buffer prune runs a full-table DELETE on every insert
Suggested fix: only prune occasionally rather than every insert (e.g. probabilistically, every Nth insert, or when a cheap Was this helpful? React with 👍 / 👎 |
||
| return row | ||
|
|
||
| async def list_recent( | ||
| self, *, level: str | None = None, limit: int = 200 | ||
| ) -> list[dict]: | ||
| """Most recent entries first, optionally filtered by level. Admin-read.""" | ||
| assert self._db is not None | ||
| limit = max(1, min(limit, 1000)) | ||
| cols = "id, user_id, level, message, source, url, stack, user_agent, created_at" | ||
| if level: | ||
| cursor = await self._db.execute( | ||
| f"SELECT {cols} FROM client_logs WHERE level = ? " | ||
| "ORDER BY created_at DESC LIMIT ?", | ||
| (level, limit), | ||
| ) | ||
| else: | ||
| cursor = await self._db.execute( | ||
| f"SELECT {cols} FROM client_logs ORDER BY created_at DESC LIMIT ?", | ||
| (limit,), | ||
| ) | ||
| rows = await cursor.fetchall() | ||
| keys = cols.split(", ") | ||
| return [dict(zip(keys, r)) for r in rows] | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SUGGESTION:
test_long_message_and_stack_are_truncatedonly coversmessageandstack. The store also capssource(200),url(1000), anduser_agent(500) — none of those truncation behaviors are tested. Add a sibling assertion that over-longsource/url/user_agentare truncated to their respectiveMAX_*_LENcaps.Reply with
@kilocode-bot fix itto have Kilo Code address this issue.