-
Notifications
You must be signed in to change notification settings - Fork 384
centralize frontend error logging and replace console.error (Refs: #1465) #1472
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
base: main
Are you sure you want to change the base?
centralize frontend error logging and replace console.error (Refs: #1465) #1472
Conversation
…n key flows\n\n- Use logging/frontend-logger to persist errors to Tauri logs\n- Replace console.error in templates, tags, search, and routes\n\nRefs: fastrepl#1465
📝 WalkthroughWalkthroughReplaces console.error calls with logFrontendError across routes, utils, and store modules. Adds the frontend logger import and passes contextual metadata (ids, query) in error paths. No control-flow or API signature changes; existing redirects/returns remain intact. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant UI as Route/Store/Utils
participant S as Service/API
participant L as FrontendLogger
U->>UI: Trigger action (create/load/sync/delete/search/tag/template)
UI->>S: Perform request/operation
alt Success
S-->>UI: Result
UI-->>U: Proceed (redirect/update/return)
else Error
S-->>UI: Error
UI->>L: logFrontendError(eventKey, error, context)
UI-->>U: Existing fallback (redirect/return/defaults)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal). Please share your feedback with us on this Discord post. 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 |
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/desktop/src/routes/app.new.tsx (2)
6-6
: Include sessionId in error contextsessionId is available; adding it tightens correlation across logs.
- logFrontendError("route.new.create_session_failed", error, { calendarEventId, record }); + logFrontendError("route.new.create_session_failed", error, { + calendarEventId, + record, + sessionId, + });Also applies to: 90-92
28-30
: Gate debug logsThese console.log calls can get noisy in production. Consider wrapping in a DEV check.
- console.log("creating a session from an event"); - console.log("event", event); + if (import.meta.env?.DEV) { + console.log("creating a session from an event", { event }); + }apps/desktop/src/utils/tag-generation.ts (1)
7-7
: Solid replacements; optional: add model/connection contextIf easy, include lightweight model/provider/connectionType metadata to aid triage (no extra try/catch).
Example (only if readily accessible without restructuring):
- logFrontendError("tags.generate_for_session_failed", error, { sessionId }); + logFrontendError("tags.generate_for_session_failed", error, { + sessionId, + // modelName, + // connectionType, + });Also applies to: 75-77, 141-143
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/desktop/src/routes/app.new.tsx
(2 hunks)apps/desktop/src/routes/app.note.$id.tsx
(4 hunks)apps/desktop/src/stores/search.ts
(2 hunks)apps/desktop/src/utils/tag-generation.ts
(3 hunks)apps/desktop/src/utils/template-service.ts
(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}
: 1. Do not add any error handling. Keep the existing one.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
apps/desktop/src/utils/tag-generation.ts
apps/desktop/src/stores/search.ts
apps/desktop/src/routes/app.new.tsx
apps/desktop/src/routes/app.note.$id.tsx
apps/desktop/src/utils/template-service.ts
🔇 Additional comments (3)
apps/desktop/src/utils/template-service.ts (1)
2-2
: LGTM; consistent error codes and safe fallbacksNice use of centralized logger with stable codes and minimal context; fallbacks remain unchanged.
Confirm frontend-logger writes only to local Tauri logs and uses rotation to avoid unbounded growth.
Also applies to: 15-18, 31-32
apps/desktop/src/routes/app.note.$id.tsx (1)
4-4
: Consistent centralized logging across load/sync/delete pathsError codes and minimal IDs are appropriate; control flow unchanged. Nice.
Confirm logFrontendError gracefully handles non-Error throwables and never throws on the UI thread.
Also applies to: 31-32, 105-106, 154-155
apps/desktop/src/stores/search.ts (1)
12-12
: Scrub search queries before logging; use centralized loggerLogging raw queries can capture PII — log a short preview + length + filter metadata instead.
- logFrontendError("search.perform_failed", error, { query }); + logFrontendError("search.perform_failed", error, { + query_preview: query.slice(0, 128), + query_length: query.length, + filter: getState().selectedTags.length > 0 ? "tag" : "text", + selected_tag_ids: getState().selectedTags.map((t: Tag) => t.id), + });
- Verified: path alias is configured — apps/desktop/tsconfig.json maps "@/" → "src/".
- Could not confirm absence of leftover console.error calls: ripgrep returned "unrecognized file type: tsx". Re-run a full search in apps/desktop/src for console.error and replace any hits with logFrontendError.
Centralizes frontend error logging via apps/desktop/src/logging/frontend-logger.ts, which writes to the Tauri logs directory (via @hypr/plugin-tracing).
Replaces remaining console.error usages in key desktop flows with logFrontendError for consistent, persistent logging.
Changes:
apps/desktop/src/utils/template-service.ts: use logFrontendError on template load failures
apps/desktop/src/utils/tag-generation.ts: use logFrontendError in tag generation paths
apps/desktop/src/stores/search.ts: use logFrontendError in debounced search
apps/desktop/src/routes/app.note.$id.tsx: use logFrontendError for session load and participant sync errors
apps/desktop/src/routes/app.new.tsx: use logFrontendError in session creation flow
Notes:
Related components already using the centralized logger include share-button-header, chat-view, and settings/views/mcp.
This aligns with issue request to add frontend error logs, not just backend.v