chore: update client submodule and enhance observability documentation
- Updated client submodule to commit 3748f0b. - Added logging standards to `docs/observability.md` to improve error handling and operational transparency. - Marked implementation checklist items as complete in `README.md` and `IMPLEMENTATION_CHECKLIST.md` for sprint GAP-012.
This commit is contained in:
2
client
2
client
Submodule client updated: 87642fabcf...3748f0b9d6
@@ -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.
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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 |
|
||||
|
||||
Reference in New Issue
Block a user