KD-0772: ship kendo-error-tracker PHP client library v0.1.0

[jasperboerhof]

Closes KD-0772

Greenfield Laravel client library that reports scrubbed exceptions into kendo's error-events endpoint (KD-0771, merged). Scaffold mirrors the sister package phpstan-warroom-rules, adapted to a runtime library (type: library, ServiceProvider auto-discovery, illuminate/* ^11–13, Pest).

Server contract (confirmed against KD-0771 PR #1408, merged to kendo main)

• Route: POST /api/projects/{project}/error-events — {project} route-key is the id (no getRouteKeyName override on the kendo Project model).
• Body: {environment, release?, exception_class, message, stack_trace} (matches StoreErrorEventRequest::rules() + IngestErrorEventData).
• Auth: Bearer; project token w/ error-events:write ability.
• Success: 202 Accepted (ErrorEventController::ingest). 401/403/422/5xx/timeout/unreachable all swallowed.

Acceptance criteria

• report($throwable) POSTs to {kendo_url}/api/projects/{project}/error-events with Bearer token + {environment, release?, exception_class, message, stack_trace} (scrubbed). No territory/context fields.
• Scrubbed payload has no JWT/Bearer/BSN/email matches (snapshot tests against fixtures).
• 202 = success; every failure (timeout, 401, 403, 422, 5xx, unreachable) writes error_log and does not throw/block.
• Async mode dispatches ReportErrorJob; sync mode POSTs inline; both swallow failure.
• base_path() strip yields identical normalized frames for /var/www/html/... and /home/forge/... (mirrors nightwatch Location::normalizeFile()).
• composer test / composer phpstan / composer format:check green (this PR's CI is the acceptance gate).

What's built

ServiceProvider (auto-discovery) → ErrorTracker (swallow-on-failure; scrubbing+normalization synchronous, before the queue boundary), ReportErrorJob (0 retries, error_log on fail, never requeue), Scrubber, PathNormalizer, publishable config/error-tracker.php (ERROR_TRACKER_*, no territory), README (install/auto-discovery/one-call integration/scrub list/config ref/token minting), CHANGELOG, CLAUDE.md. CI (ci.yml PR+push main) + release.yml (tag→GitHub release; packagist push-sync).

Verification (local)

• composer test → 26 passed (52 assertions): ScrubberTest, PathNormalizerTest, ReportTest (HTTP::fake), SwallowTest (every failure mode).
• composer phpstan → level max, no errors.
• composer format:check → passed (Pint, canonical war-room config).
• composer audit → no advisories.

Deviations from kendo / warroom

• Reads config live from the Config repository (not a constructor snapshot) so a singleton bound at register() always sees current config — needed because the consuming app sets config after provider registration.
• ErrorTracker::configString() narrows the repository's mixed return for PHPStan level max (the package does not pull Larastan).
• CI drops warroom's mutation/coverage gates (runtime library, not a rules package); keeps test/phpstan/format/audit.

🤖 Generated with Claude Code

[jasperboerhof]

PR Reviewer · claimed

• Target: kendo-error-tracker
• PR: KD-0772: ship kendo-error-tracker PHP client library v0.1.0 #1
• Branch: KD-0772-php-client-library
• AC anchor: kendo-issue (KD-0772)
• Worktree: ~/Code/agent-worktrees/kendo-error-tracker/review-1

[jasperboerhof]

PR Reviewer · 6/10 · REVISE — 🔴 2 🟡 3

kendo-error-tracker #1 · AC anchor: kendo-issue KD-0772
Scores: acceptance 9 · simplicity 9 · surface 6 · silent-failure 6 · efficiency –

🔴 MAJOR

src/ErrorTracker.php:75 — Outbound error-report POST sets no explicit timeout, undercutting the never-blocks-the-caller contract

why + fix

send() builds the request as ->withToken()->acceptJson()->asJson()->post() (ErrorTracker.php:75-79) with no ->timeout()/->connectTimeout(). It therefore inherits Laravel's 30s request / 10s connect default. The kendo cross-project convention pinned in repos/kendo/backend/tests/Arch/ExternalHttpTimeoutTest.php (Doctrine Principle #​8 / ADR-0024 territory: 'All outbound HTTP requests to external services must have an explicit timeout set. No relying on PHP/framework defaults. Per-call timeouts are preferred over a global default') requires an explicit value. A 30s default block contradicts this library's stated 'never blocks the caller' guarantee in sync mode and pins a queue worker for 30s in async mode. The README (L125) and the send() docblock (ErrorTracker.php:61) both advertise 'HTTP timeout' as a swallowed failure mode, but as written that mode is only reachable via a network-level reset (ConnectionException) — a slow-but-connected kendo server will hold the caller for the full 30s default rather than failing fast. This is the 'missing timeout citation' MAJOR named in the Row 2 rubric. Not a BLOCKER only because Laravel does bound the default at 30s rather than leaving it unbounded.

Fix: Add a short, config-tunable ->timeout(...)->connectTimeout(...) to the request chain in send() (a few seconds suits fire-and-forget telemetry), exposed as an error-tracker.timeout config key with an env default.

src/ErrorTracker.php:75 — HTTP POST has no timeout(); in sync mode a hung kendo host silently blocks the caller, breaking the documented "never blocks" guarantee

why + fix

send() builds the request as $this->http->withToken(...)->acceptJson()->asJson()->post($url, $payload) with no ->timeout() or ->connectTimeout() — confirmed by grep across src/ and config/ (only a docblock mentions the word "timeout"). The class docblock (lines 22-24) and report()'s contract explicitly promise "never throws and never blocks the caller" (KD-0772). The exception is correctly swallowed, but the blocking is not bounded: Laravel's HTTP client defaults to a 30s read timeout and an OS-default connect timeout that, against a black-holed/firewalled host, can hang well past 30s. In sync mode (error-tracker.sync=true) this POST runs inline in the caller's request thread, so a slow or unreachable kendo endpoint silently adds up to ~30s of latency to every erroring request — with nothing logged until the timeout finally fires. This is the worst-case coupling for an error tracker, which is most likely to be exercised during an incident when the kendo endpoint may itself be degraded. In async mode it instead ties up a queue worker for ~30s per hung report. This is prose-vs-code drift in the silent-failure lane: the stated guarantee is contradicted by the missing bound. SwallowTest covers throw-suppression but never asserts a latency bound, so the suite is green while the block is unbounded — don't take "swallow tested" as covering this.

Fix: Set an explicit tight timeout on construction of the request, e.g. ->connectTimeout((float) $this->config->get('error-tracker.connect_timeout', 2))->timeout((float) $this->config->get('error-tracker.timeout', 5)) before ->post(...), and add the two keys to config/error-tracker.php with conservative defaults (connect 2s, total 5s). Add a SwallowTest case asserting send() returns promptly on a hung host (Http::fake with a delayed/throwing closure).

🟡 MINOR

src/ErrorTracker.php:105 — buildPayload reads error-tracker.release twice (raw null-check, then configString re-read of the same key)

why + fix

Line 105 reads error-tracker.release raw to null-check; line 109 re-reads the SAME key via configString('release') to stringify, discarding the first read's value. I traced whether the double-read is load-bearing: it IS — array_filter only strips null, not '', and configString collapses a missing key to ''. So the $release === null ? null : ... ternary is the mechanism that turns an unset release into null (then dropped) versus an empty '' that would otherwise be sent, violating KD-0771's release? omit-when-unset contract. So this is correct, not dead code — the only nit is the same config key is fetched twice rather than the raw $release being reused (reuse would force re-narrowing mixed→string). Low-confidence and arguably the clearest expression; naming it per the skeptic discipline rather than omitting. Everything else in the diff is the simplest shape that meets the contract.

Fix: Optional only: $release = $this->configString('release'); ... 'release' => $release === '' ? null : $release collapses to a single read. Leave as-is if the current null-vs-empty intent reads clearer — this does not block.

phpstan.neon.dist — No L1 arch test pins the HTTP-client timeout convention this package introduces

why + fix

Row 6 / ADR-0021 enforcement-ladder gap. The package introduces the structural rule 'the outbound HTTP client declares a timeout' but nothing enforces it at CI time — the kendo precedent (ExternalHttpTimeoutTest.php) is exactly the L1 transplant that is missing. Same root cause as the Row 2 MAJOR; recorded once here as the enforcement-level gap, not double-counted in severity. Without it a future send-path method can silently regress the timeout convention.

Fix: Add a small Pest arch test asserting src/ErrorTracker.php contains '->timeout(', mirroring kendo's ExternalHttpTimeoutTest named-class whitelist (L1 on the Escalation Ladder).

src/ErrorTracker.php:124 — Missing kendo_url/project/token silently collapses to empty strings → malformed request, no operator signal that tracking is disabled

why + fix

configString() narrows any non-scalar/null config to '' (line 128). With kendo_url/project/token unset (the test 'does not throw when configuration is entirely missing' exercises exactly this), report() builds a POST to {empty}/api/projects//error-events with an empty Bearer token, which either fails to resolve or returns a generic 404/422 logged only as 'send rejected: HTTP '. Swallow-on-failure is the documented contract, so this is not a runtime silent failure of a user flow — but a misconfigured install silently drops every error report with no distinct, actionable signal that error-tracking is unconfigured. An on-call engineer wiring this into a new app would see nothing telling them the token/url is missing.

Fix: Emit one distinct error_log line when a required key (kendo_url/project/token) is empty at send time — e.g. error_log('[kendo-error-tracker] not configured: missing kendo_url/project/token; report dropped') — so a misconfigured install is diagnosable. Optionally short-circuit send() before the HTTP call when any required key is empty, to avoid the malformed outbound request entirely.

Action

revise — address MAJORs

[jasperboerhof]

PR Reviewer · claimed

• Target: kendo-error-tracker
• PR: KD-0772: ship kendo-error-tracker PHP client library v0.1.0 #1
• Branch: KD-0772-php-client-library
• AC anchor: kendo-issue (KD-0772)
• Worktree: ~/Code/agent-worktrees/kendo-error-tracker/review-1

[jasperboerhof]

PR Reviewer · 9/10 · PASS — 🟡 3

kendo-error-tracker #1 · AC anchor: kendo-issue KD-0772
Scores: acceptance 9 · simplicity 9 · surface 9 · silent-failure 9 · efficiency –

🟡 MINOR

src/ErrorTracker.php:124-128 — buildPayload reads error-tracker.release twice and threads it through a redundant ternary

why + fix

$release = $this->config->get('error-tracker.release') reads the key raw, then 'release' => $release === null ? null : $this->configString('release') reads it a SECOND time via configString. Since configString() already collapses null to '' and array_filter() already drops nulls, the raw read + the ternary guard are doing work the helpers + filter already do. Not a bug — ReportTest covers both the present-release and null-release cases and they pass — purely a readability/lean nit. (Plan-less mode: no DECISIONS.md sanctions this shape, so it is judged on universal simplicity.)

Fix: Read once: 'release' => $this->configString('release'), and have the array_filter drop empty strings as well as nulls ($value !== null && $value !== ''), OR keep a single $this->config->get(...) read and inline the null check. Either removes the double read and the nested-conditional.

src/Scrubber.php:24 — Scrubber is a four-pattern denylist; exception_class is sent unscrubbed

why + fix

The scrubber is an explicit denylist covering exactly the four named contract classes — JWT, Bearer, BSN, email — and all four are implemented correctly and tested (ScrubberTest.php). The named PII contract from KD-0772 is therefore MET, which is why this is MINOR/informational, not a defect. Two scope notes for the reader, not blockers: (1) the docstring and CHANGELOG honestly disclose this as a 'v1 pattern set [covering] the leaks seen most often' — other secret classes (DB-connection passwords e.g. 'password=...', non-Bearer API keys, credit cards) fall outside the stated scope and are not redacted; (2) the exception_class field (ErrorTracker.php:129, $throwable::class) bypasses scrub() entirely — safe today because it is always a PHP FQCN that cannot carry PII, but if a future change ever routes a non-class string into that field it would leak unscrubbed. getTraceAsString() truncates scalar args, so trace-arg leakage is bounded and any surviving JWT/email/BSN run is still caught. The summary's own conclusion — named contract satisfied, limitation disclosed — is why severity is MINOR, not MAJOR.

Fix: No change required for merge — the named contract is met. Optional hardening for a later iteration: add an arch-test or doc note pinning that exception_class must remain class-name-only, and track the denylist's known-out-of-scope classes (DB passwords / generic api-key= forms) as a follow-up so the v2 pattern set is a deliberate decision rather than a silent gap.

tests/Unit/ScrubberTest.php:1 — Stack-trace scrubbing path (highest-PII-risk surface) has no dedicated test

why + fix

buildPayload() scrubs BOTH the message and the path-normalized stack trace (src/ErrorTracker.php:119-122), but every PII test (ScrubberTest, ReportTest 'scrubs the message') exercises only the message string. The stack trace is the higher-risk leak surface — getTraceAsString() can embed method arguments, and the trace is scrubbed AFTER PathNormalizer runs, so the scrub-on-normalized-string ordering is untested. The code path is correct; only its coverage is thin. Not a silent failure — the scrubbing genuinely runs — so MINOR. Falsifier that would have flipped this to OK: a test planting a BSN/JWT into a Throwable trace (or feeding a normalized trace through buildPayload) and asserting the outbound stack_trace carries the [REDACTED:*] marker.

Fix: Add a Pest case that throws an exception whose trace would contain a scrubable token and asserts the outbound stack_trace key contains [REDACTED:*] and not the raw value — closing the gap between 'message is scrubbed' coverage and the equally-load-bearing trace path.

Action

merge-ready

⚠ Approval withheld — self-authored (GitHub forbids self-approval)