feat(operability): structured server diagnostics behind the generic 503 - #577
Open
seonghobae wants to merge 3 commits into
Open
feat(operability): structured server diagnostics behind the generic 503#577seonghobae wants to merge 3 commits into
seonghobae wants to merge 3 commits into
Conversation
Global Ask hid every failure behind a stable 503 (correct customer boundary) but the cause never reached structured telemetry, and the f-string leaked raw exception text to callers. Split the ask handler into three classified paths, all returning the same generic 503: - HttpClientError/OSError -> known provider/transport fault; warning event orchestrator_provider_unavailable with operation code, correlation id, exception class; message deliberately not logged. - KeyError/ValueError -> evidence-object contract break; error event orchestrator_internal_fault with stack trace attached. - broad Exception -> unexpected defect; same internal-fault diagnostic so a programming regression cannot degrade into an opaque availability incident. Chaining is preserved on every path. backend/app/operability.py documents the forbidden-field contract (no prompt text, model output, bearer tokens, provider keys, tenant PII, or post bodies in any record); alerting keys on event_type so pager load separates provider-down from our-bug (issue #361). Unit tests cover both event shapes, uniqueness of correlation ids, and the forbidden- field guarantee without needing a live stack.
|
Warning Review limit reachedNext included review available in 43 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthrough오케스트레이터 오류를 구조화해 기록하고 클라이언트 응답에서 예외 세부 내용을 제거합니다. PostgreSQL 레거시 네임스페이스를 canonical 형식으로 변환하는 안전한 마이그레이션 CLI와 테스트를 추가합니다. Changes오케스트레이터 오류 처리
레거시 네임스페이스 마이그레이션
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🟠 High · up to The current change may expose different internal failure details through the 503 response and may leak sensitive exception text into diagnostic logs, potentially including prompt or credential-related data. Merge should wait until every failure path uses the same generic response and logged tracebacks are sanitized. Sequence Diagram(s)sequenceDiagram
participant GlobalAskAgent
participant OperabilityLogger
participant Client
GlobalAskAgent->>OperabilityLogger: 오류 유형별 구조화된 이벤트 기록
OperabilityLogger-->>GlobalAskAgent: correlation_id 반환
GlobalAskAgent-->>Client: 일반화된 503 응답 반환
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
ADR 0157 keeps the lowercase ontology namespace canonical and demoted the repository-case spelling to deprecated compatibility status, but rows written before the decision can still carry legacy IRIs in post_project_mention.ontology_iri -- and RDF consumers treat the two spellings as different resources. scripts/migrate_legacy_namespace.py scans, prints every planned rewrite, refuses unrecognized namespaces (fail closed rather than bulk-mangle a third spelling), and only writes under --apply inside one transaction guarded by the exact old IRI so a concurrent edit aborts instead of double-applying. Provenance columns (extraction method, confidence, evidence) are never touched per ADR 0157's do-not-silently-rewrite rule. Dry run is the default. 8 unit tests cover canonicalize mapping, dry-run reporting, selective apply, fail-closed behavior, and the clean-database no-op.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
seonghobae
enabled auto-merge
August 24, 2026 09:12
…backs Python renders a traceback's final line as 'ExceptionType: message', so exc_info=exc violated the module's own forbidden-field contract: parsing exceptions from orchestrator responses can embed provider payload or prompt fragments, and those landed in the log. Re-emit through a _MessageRedacted carrier that keeps the original __traceback__ (the raise-site frames stay diagnosable) while its text is a fixed redaction notice; the real class name is retained in the structured exception_class field. Also unify the three /api/ask 503 detail strings into one generic message so callers cannot probe which internal classifier fired; the provider-vs-contract-vs-defect distinction lives only in server-side event_type (devin/coderabbit review threads on PR #577). Drop the docstring's nonexistent include_message option.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements #361 for the /api/ask boundary.
Customer boundary unchanged
Every failure path still returns the same generic 503 — no exception text, provider trace, or prompt/response content reaches the caller. The old f-string detail (
Ask Agent is unavailable: {exc}) leaked raw exception text and is gone.Operator diagnosability restored
Three classified catch paths in
/api/ask:orchestrator_provider_unavailable(operation, correlation id, exception class; message deliberately excluded)orchestrator_internal_faultwith stack traceNew
backend/app/operability.pydocuments the forbidden-field contract: no prompt text, model output, bearer tokens, provider keys, tenant PII, or post bodies in any record. Alerting keys onevent_typeso pager load separates provider-down from our-bug.Tests
tests/test_operability.py(5 cases, no live stack needed): event shapes, correlation-id uniqueness, stack-trace attachment for internal faults, forbidden-field guarantee.Follow-up commit: legacy-namespace migration tooling (ADR 0157 / #372)
scripts/migrate_legacy_namespace.py+ 8 tests: deterministic, dry-run-by-default rewrite of storedpost_project_mention.ontology_irivalues from the deprecated repository-case namespace to the canonical lowercase one. Fails closed on unrecognized namespaces; provenance columns untouched.Summary by CodeRabbit