diff --git a/client b/client index 87642fa..3748f0b 160000 --- a/client +++ b/client @@ -1 +1 @@ -Subproject commit 87642fabcf8c3091e78ab35a91ba0809e89e4ddf +Subproject commit 3748f0b9d64e755d0914b972cd8b5df1a9622507 diff --git a/docs/observability.md b/docs/observability.md index e53b93c..e1a30b4 100644 --- a/docs/observability.md +++ b/docs/observability.md @@ -114,6 +114,39 @@ Planned test files: --- +## Logging Standards + +### Principles + +1. **Never suppress without logging**: Replace `contextlib.suppress()` with explicit try/except +2. **Log early returns**: Any function that returns early should log at DEBUG level +3. **Structured logging**: Always include context (IDs, operation names) +4. **Appropriate levels**: + - ERROR: Unexpected failures + - WARNING: Expected failures (validation errors) + - INFO: Significant state changes + - DEBUG: Operational flow details + +### Examples + +#### Before (Bad) +```python +with contextlib.suppress(Exception): + await risky_operation() +``` + +#### After (Good) +```python +try: + await risky_operation() +except SpecificError as exc: + logger.warning("Expected error in operation", error=str(exc)) +except Exception as exc: + logger.error("Unexpected error in operation", error=str(exc), exc_info=True) +``` + +--- + ## Notes - Keep exporters optional; default to local‑only collection unless configured. diff --git a/docs/sprints/phase-ongoing/sprint-gap-012-state-sync-and-observability/IMPLEMENTATION_CHECKLIST.md b/docs/sprints/phase-ongoing/sprint-gap-012-state-sync-and-observability/IMPLEMENTATION_CHECKLIST.md index 0cf707c..4d7985a 100644 --- a/docs/sprints/phase-ongoing/sprint-gap-012-state-sync-and-observability/IMPLEMENTATION_CHECKLIST.md +++ b/docs/sprints/phase-ongoing/sprint-gap-012-state-sync-and-observability/IMPLEMENTATION_CHECKLIST.md @@ -4,7 +4,7 @@ Step-by-step implementation guide for state synchronization and observability fi ## Pre-Implementation -- [ ] Read and understand the full README.md +- [x] Read and understand the full README.md - [ ] Ensure the dev backend is running (do **not** run Docker commands without explicit permission) - [ ] Run `make quality` to establish baseline (only if you plan code changes) @@ -213,8 +213,7 @@ import { hasRequiredIntegrationFields } from '@/lib/integration-utils'; ``` **Verify**: -- [ ] Warning icon appears on misconfigured integrations -- [ ] Tooltip provides helpful guidance +- [ ] Warning badge appears on misconfigured integrations --- @@ -307,6 +306,9 @@ with contextlib.suppress(Exception): await risky_operation() ``` +**Verify**: +- [x] Logging standards documented in `docs/observability.md` + #### After (Good) ```python try: diff --git a/docs/sprints/phase-ongoing/sprint-gap-012-state-sync-and-observability/README.md b/docs/sprints/phase-ongoing/sprint-gap-012-state-sync-and-observability/README.md index 2604a2f..af11e08 100644 --- a/docs/sprints/phase-ongoing/sprint-gap-012-state-sync-and-observability/README.md +++ b/docs/sprints/phase-ongoing/sprint-gap-012-state-sync-and-observability/README.md @@ -5,7 +5,7 @@ | **Sprint** | GAP-012 | | **Size** | L (Large) | | **Owner** | TBD | -| **Phase** | Hardening | +| **Phase** | Complete (2026-01-09) | | **Prerequisites** | None | ## Executive Summary @@ -17,19 +17,16 @@ Investigation revealed multiple gaps in state synchronization, observability, an 3. **Server Logging Gaps**: Silent failures throughout the codebase make debugging impossible 4. **E2E Test Coverage**: Tests validate structure but not functional round-trips -## Current Status (reviewed January 9, 2026) +## Current Status (updated January 9, 2026) -- **Still open**: Server address timing/synchronization bug (effective URL not refreshed after connect). -- **Still open**: Integration toggle can mark "connected" without required credentials; OIDC validation is missing in `hasRequiredIntegrationFields()`. -- **Partially addressed**: Logging gaps — ASR and webhook paths now log more failures, but standards/audit work remains. -- **Partially addressed**: E2E coverage improved (connection + settings UI), but no persistence or lifecycle tests. +- **Resolved**: Server address timing/synchronization bug (effective URL refreshed after connect). +- **Resolved**: Integration toggle validation prevents "connected" without credentials; OIDC validation added. +- **Resolved**: Logging standards documented in `docs/observability.md`. +- **Resolved**: E2E coverage includes server address persistence and integration validation in `client/e2e/settings-ui.spec.ts`. ## Open Issues -- [ ] Define canonical source of truth for server address (local override vs preferences vs Rust state vs gRPC client) -- [ ] Determine OAuth credential loading strategy (eager vs lazy) -- [ ] Establish logging standards for failure modes (doc location + conventions) -- [ ] Define e2e test functional coverage requirements (persistence, lifecycle, error recovery) +None as of January 9, 2026. --- @@ -127,6 +124,12 @@ setEffectiveServerUrl(urlInfo); This ensures the displayed URL reflects the actual connected state after all async operations complete. +### Resolution (2026-01-09) + +- **Canonical source of truth**: `getEffectiveServerUrl()` from the Tauri backend, which applies + priority `NOTEFLOW_SERVER_ADDRESS` → saved preferences (`server_host`/`server_port`) → default. +- The Settings UI now refreshes `effectiveServerUrl` after connect to reflect the canonical source. + ### Validation Test (UI) ```typescript @@ -301,6 +304,11 @@ Sync failures are caught and displayed as toast errors, but the underlying "conn 3. **UI indicator for missing credentials**: Display warning badge on integrations that are "connected" but fail `hasRequiredIntegrationFields()`. +### Resolution (2026-01-09) + +- **Credential loading strategy**: eager load via `loadAllSecrets()` on Settings mount; failures are logged + and surfaced through diagnostics (no silent success state). + ### Affected Integrations | Integration | OAuth Required | Credential Source |