-
Notifications
You must be signed in to change notification settings - Fork 537
Persist log to file at runtime #1438
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
Conversation
📝 WalkthroughWalkthroughAdds a new Tauri tracing plugin (crate + JS package), wires it into the desktop app replacing manual tracing setup, updates Tauri/plugin versions, adds a tracing capability permission, introduces a logs command/extension, and adjusts tray/about and help UI to surface logs access. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Desktop App
participant Tauri Core
participant Tracing Plugin
participant File Logger
participant Sentry
User->>Desktop App: Launch
Desktop App->>Tauri Core: build()
Desktop App->>Tracing Plugin: init()
Note right of Tracing Plugin: Configure EnvFilter, fmt layer, Sentry layer, optional file appender
alt File logging enabled
Tracing Plugin->>File Logger: create daily rotating writer (non-blocking)
else Console only
Tracing Plugin-->>Desktop App: attach console subscriber
end
Tracing Plugin-->>Tauri Core: plugin registered
Desktop App-->>User: ready
par Runtime tracing
Tauri Core->>Tracing Plugin: emit spans/events
Tracing Plugin->>Sentry: forward errors/breadcrumbs
Tracing Plugin->>File Logger: write log entries
end
sequenceDiagram
autonumber
participant Frontend
participant Tauri Core
participant Tracing Cmd
participant AppHandle
participant Tracing Ext
Frontend->>Tauri Core: invoke("logsDir")
Tauri Core->>Tracing Cmd: commands::logs_dir(app)
Tracing Cmd->>Tracing Ext: app.logs_dir(bundle_id)
Tracing Ext-->>Tracing Cmd: PathBuf (logs dir)
Tracing Cmd-->>Tauri Core: Ok(path)
Tauri Core-->>Frontend: return path
Frontend->>OS: openPath(path)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
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 (15)
plugins/tracing/js/index.ts (1)
1-1: If bindings.gen has a default export, re-export it too.
Prevents breaking consumers expecting a default.export * from "./bindings.gen"; +export { default } from "./bindings.gen";plugins/tracing/tsconfig.json (1)
3-3: Widen include to future subfolders.
Covers nested files without future tsconfig churn.- "include": ["./js/*.ts"], + "include": ["./js/**/*.ts"],plugins/tracing/.gitignore (1)
7-9: Add pnpm lockfile ignore (if repo uses pnpm).
Keeps lockfile noise out of the plugin package.debug.log package-lock.json .vscode/settings.json yarn.lock +pnpm-lock.yamlplugins/tray/src/ext.rs (1)
139-144: Improve About dialog readability with sub-bullets for backends.
Keeps list visually aligned with the other bullets.- let backends_formatted = app_backends - .iter() - .map(|b| format!("{:?}", b)) - .collect::<Vec<_>>() - .join("\n"); + let backends_formatted = app_backends + .iter() + .map(|b| format!(" • {:?}", b)) + .collect::<Vec<_>>() + .join("\n"); - let message = format!( - "• App Name: {}\n• App Version: {}\n• SHA:\n {}\n• Backends:\n{}", - app_name, app_version, app_commit, backends_formatted - ); + let message = format!( + "• App Name: {}\n• App Version: {}\n• SHA:\n {}\n• Backends:\n{}", + app_name, app_version, app_commit, backends_formatted + );Also applies to: 146-148
plugins/tracing/src/errors.rs (2)
3-4: Empty error type makes Result<_, Error> uninhabited — is this intentional?With
pub enum Error {}you can’t construct an error; anyResult<T, Error>can never beErr. If that’s the goal, consider usingcore::convert::Infalliblefor clarity; otherwise add variants later.
6-13: Avoid String allocation in Serialize implUse the serializer’s
collect_strto format via Display without allocating.impl Serialize for Error { fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error> where S: Serializer, { - serializer.serialize_str(self.to_string().as_ref()) + serializer.collect_str(self) } }plugins/tracing/package.json (1)
1-11: Add a prepare script to generate bindings
In plugins/tracing/package.json, under “scripts” add:"prepare": "npm run codegen"This guarantees that
bindings.gen.ts(currently missing from plugins/tracing/js) is generated before install/build, preventing import errors in index.ts.plugins/tracing/src/commands.rs (2)
1-7: Use the crate’s error type instead of String for consistencyYou already define
crate::Error. Returning it improves typing across Rust/TS via specta.-use crate::TracingPluginExt; +use crate::TracingPluginExt; #[tauri::command] #[specta::specta] -pub async fn hi<R: tauri::Runtime>(app: tauri::AppHandle<R>) -> Result<(), String> { - app.hi().map_err(|e| e.to_string()) +pub async fn hi<R: tauri::Runtime>(app: tauri::AppHandle<R>) -> Result<(), crate::Error> { + app.hi() }
1-7: Remove unusedhicommand and trait
No call sites forhiexist; drop thepub async fn hiinplugins/tracing/src/commands.rsand the correspondinghimethod inplugins/tracing/src/ext.rsto eliminate dead API.plugins/tracing/src/ext.rs (1)
1-9: Placeholder extension method—either implement a real action or remove
hi()returnsOk(())and does nothing. Either implement a meaningful operation (e.g., rotate logs, return log file path) or remove to reduce surface area.-pub trait TracingPluginExt<R: tauri::Runtime> { - fn hi(&self) -> Result<(), crate::Error>; -} - -impl<R: tauri::Runtime, T: tauri::Manager<R>> TracingPluginExt<R> for T { - fn hi(&self) -> Result<(), crate::Error> { - Ok(()) - } -} +pub trait TracingPluginExt<R: tauri::Runtime> {} +impl<R: tauri::Runtime, T: tauri::Manager<R>> TracingPluginExt<R> for T {}plugins/tracing/Cargo.toml (3)
1-9: Package metadata is minimal; keep or expand intentionallyIf this is internal-only, fine. If publishing later, add
license,repository, and a shortdescription.
16-21: Drop unnecessary tauri features to reduce compile time/binary sizeThe tracing plugin likely doesn’t need
tray-iconorimage-png.-tauri = { workspace = true, features = ["tray-icon", "image-png"] } +tauri = { workspace = true }
28-30: Remove unusedserde_jsondependency
No references toserde_jsonwere found inplugins/tracing/src; safely remove the line in Cargo.toml. Other deps (dirs,tracing-appender,tracing-subscriber) are confirmed in use.plugins/tracing/src/lib.rs (2)
8-15: Prefer the public prelude import for readability.Avoid the internal path
__tracing_subscriber_SubscriberExt.Apply:
-use tracing_subscriber::{ - fmt, prelude::__tracing_subscriber_SubscriberExt, util::SubscriberInitExt, EnvFilter, -}; +use tracing_subscriber::{fmt, prelude::*, util::SubscriberInitExt, EnvFilter};
61-82: Use Tauri’s path resolver instead ofdirs::data_dirto follow platform conventions and config changes.Resolves the base directory from the app, not global OS dirs + bundle id.
Apply:
-fn make_file_writer_if_enabled( - enabled: bool, - bundle_identifier: &str, -) -> Option<(tracing_appender::non_blocking::NonBlocking, WorkerGuard)> { +fn make_file_writer_if_enabled( + enabled: bool, + app: &tauri::AppHandle<tauri::Wry>, +) -> Option<(tracing_appender::non_blocking::NonBlocking, WorkerGuard)> { @@ - let base_dir: PathBuf = match dirs::data_dir() { - Some(p) => p, - None => return None, - }; - - let logs_dir = base_dir.join(bundle_identifier).join("logs"); + let base_dir: PathBuf = match app.path().app_data_dir() { + Some(p) => p, + None => return None, + }; + let logs_dir = base_dir.join("logs");And at call site:
- if let Some((file_writer, guard)) = make_file_writer_if_enabled(true, &bundle_id) { + if let Some((file_writer, guard)) = make_file_writer_if_enabled(true, app) {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (8)
Cargo.lockis excluded by!**/*.lockplugins/tracing/js/bindings.gen.tsis excluded by!**/*.gen.tsplugins/tracing/permissions/autogenerated/commands/hi.tomlis excluded by!plugins/**/permissions/**plugins/tracing/permissions/autogenerated/commands/ping.tomlis excluded by!plugins/**/permissions/**plugins/tracing/permissions/autogenerated/reference.mdis excluded by!plugins/**/permissions/**plugins/tracing/permissions/default.tomlis excluded by!plugins/**/permissions/**plugins/tracing/permissions/schemas/schema.jsonis excluded by!plugins/**/permissions/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
Cargo.toml(3 hunks)apps/desktop/package.json(1 hunks)apps/desktop/src-tauri/Cargo.toml(1 hunks)apps/desktop/src-tauri/capabilities/default.json(1 hunks)apps/desktop/src-tauri/src/lib.rs(1 hunks)plugins/tracing/.gitignore(1 hunks)plugins/tracing/Cargo.toml(1 hunks)plugins/tracing/build.rs(1 hunks)plugins/tracing/js/index.ts(1 hunks)plugins/tracing/package.json(1 hunks)plugins/tracing/src/commands.rs(1 hunks)plugins/tracing/src/errors.rs(1 hunks)plugins/tracing/src/ext.rs(1 hunks)plugins/tracing/src/lib.rs(1 hunks)plugins/tracing/tsconfig.json(1 hunks)plugins/tray/src/ext.rs(1 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:
plugins/tracing/src/commands.rsplugins/tracing/src/ext.rsplugins/tray/src/ext.rsplugins/tracing/js/index.tsplugins/tracing/build.rsapps/desktop/src-tauri/src/lib.rsplugins/tracing/src/lib.rsplugins/tracing/src/errors.rs
🧬 Code graph analysis (5)
plugins/tracing/src/commands.rs (1)
plugins/tracing/src/ext.rs (2)
hi(2-2)hi(6-8)
plugins/tracing/src/ext.rs (1)
plugins/tracing/src/commands.rs (1)
hi(5-7)
plugins/tray/src/ext.rs (1)
crates/notification-interface/src/lib.rs (1)
message(36-39)
apps/desktop/src-tauri/src/lib.rs (1)
plugins/tracing/src/lib.rs (1)
init(26-59)
plugins/tracing/src/lib.rs (4)
apps/desktop/src-tauri/src/lib.rs (3)
make_specta_builder(339-351)make_specta_builder(359-359)export_types(358-368)plugins/tracing/src/commands.rs (1)
hi(5-7)plugins/tracing/src/ext.rs (2)
hi(2-2)hi(6-8)owhisper/owhisper-config/src/lib.rs (1)
data_dir(52-54)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ci (macos, macos-latest)
- GitHub Check: ci (windows, windows-latest)
🔇 Additional comments (18)
plugins/tracing/js/index.ts (1)
1-1: LGTM: minimal re-export is correct.plugins/tracing/build.rs (2)
1-5: LGTM: build script wires plugin commands correctly.
1-5: COMMANDS array is up-to-date
The only#[tauri::command]function (hi) is already listed inCOMMANDS.plugins/tray/src/ext.rs (1)
155-158: Confirm dialog boolean maps to primary “Copy”.
Ensureresult == truecorresponds to the “Copy” button for all platforms.apps/desktop/package.json (1)
38-38: Missing imports and generated bindings for @hypr/plugin-tracingNo TS/TSX imports or
bindings.gen.tsfiles were found in the repo. Manually verify that:
- the desktop app actually imports/uses
@hypr/plugin-tracingat runtime, and- the TypeScript bindings (e.g.
plugins/tracing/js/bindings.gen.ts) are generated as part of your dev/build process.If either is missing, add the import and bindings generation or remove the unused dependency.
Cargo.toml (3)
122-122: Path dep name consistency Verified thatplugins/tracing/Cargo.tomldefinesname = "tauri-plugin-tracing", matching the workspace dependency. No changes needed.
100-100: No duplicatesentryor conflicting features detected –tauri-plugin-sentry = "0.4.1"andsentry = "0.38.1"are only declared in the root Cargo.toml; workspace members reference the plugin viaworkspace = truewithout re-declaringsentry.
88-92: Tauri core/plugin version matrix sanity checkConfirm that Cargo.lock resolves all Tauri‐related crates to compatible versions (core at 2.8.x, build/runtime at 2.8.x, plugins at 2.4.x, clipboard-manager at 2.3.x, nsPanel from the v2 branch). For example, run:
rg -nE -C1 '^name = "(tauri|tauri-build|tauri-nspanel|tauri-plugin|tauri-plugin-clipboard-manager)"$' Cargo.lockand verify each adjacent
version = "…"line matches the intended ranges.apps/desktop/src-tauri/capabilities/default.json (1)
97-99: Confirm permission entries match actual usage
- Verify
tracing:defaultis declared in thetracingplugin and actually invoked by the app.- Ensure the plugin’s log file path is covered by existing
fs:allow-write-filerules (e.g.$APPDATA/**).- Check that
dialog:allow-saveis used in the UI; remove it if it’s unused.apps/desktop/src-tauri/Cargo.toml (2)
53-53: Tracing plugin dependency wiring looks correctAdding
tauri-plugin-tracing = { workspace = true }aligns with the new plugin crate and keeps versions centralized. No issues.
59-75: Plugin versions align with Tauri core 2.8
All direct semver-pinned plugins (autostart 2.5, global-shortcut 2.3, os 2.3, process 2.3, updater 2.9, single-instance 2.x) share major version 2 with the Tauri core (2.8). Workspace-managed plugins are local workspace members and will use matching versions.apps/desktop/src-tauri/src/lib.rs (2)
63-63: Good placement: tracing plugin early in the chainInitializing tracing before most plugins ensures early logs are captured. Also Sentry is initialized before build, so the Sentry tracing layer will bind correctly.
63-63: No conflicting global subscriber initializations detected
Scanned all Rust crates—no other calls totracing_subscriber::…init()orSubscriber::init()were found.plugins/tracing/src/lib.rs (5)
1-7: Module surface and re-exports look good.Clean structure; public exports are purposeful.
18-24: Specta builder wiring LGTM.Namespacing and error handling mode are appropriate.
40-46: Dual fmt layers to tee console and file outputs is correct.Console keeps ANSI; file disables ANSI. Nice.
26-59: Replace panic-inducingassert!with a non-panickingmanagecall
Changeassert!(app.manage(guard));to
let _ = app.manage(guard);to avoid a startup panic if the state already exists while still keeping the guard alive. Verified no other
tracing_subscriber::…init()paths in the repo.
84-100: Keep.formatter(prettier)and the./js/bindings.gen.tspath
There is noformatter::noneinspecta_typescript::formatter, and every plugin crate already commits ajs/directory—so the test isn’t fragile as written.Likely an incorrect or invalid review 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 (6)
crates/detect/src/list.rs (1)
47-73: Good: helper gated with the same target; consider consolidating cfg to reduce repetitionThe private helper is correctly macOS-only. If this pattern spreads, consider a small module to centralize gating and re-export the public API.
Example outline (illustrative, no behavior change):
+#[cfg(target_os = "macos")] +mod macos_impl { + use super::*; + // existing macOS implementations here +} +#[cfg(target_os = "macos")] +pub use macos_impl::list_installed_apps;crates/whisper-local/src/model.rs (1)
173-176: Use character count instead of byte length for "text_length".
len()on a UTF-8Stringcounts bytes, not user-perceived characters. If you intended characters, switch to.chars().count().- tracing::info!(text_length = full_text.len(), "transcribe_completed"); + tracing::info!(text_length = full_text.chars().count(), "transcribe_completed");plugins/tracing/src/commands.rs (1)
5-10: Avoid unnecessary clone and async.This command is sync and doesn’t await; also you can borrow the identifier to skip the clone.
-#[tauri::command] -#[specta::specta] -pub async fn logs_dir<R: tauri::Runtime>(app: tauri::AppHandle<R>) -> Result<PathBuf, String> { - let bundle_id = app.config().identifier.clone(); - app.logs_dir(bundle_id).map_err(|e| e.to_string()) -} +#[tauri::command] +#[specta::specta] +pub fn logs_dir<R: tauri::Runtime>(app: tauri::AppHandle<R>) -> Result<PathBuf, String> { + let bundle_id = &app.config().identifier; + app.logs_dir(bundle_id).map_err(|e| e.to_string()) +}plugins/tracing/src/ext.rs (1)
8-12: Document why unwrap is acceptable here.Add a brief “why” comment to justify
unwrap()in supported environments.impl<R: tauri::Runtime, T: tauri::Manager<R>> TracingPluginExt<R> for T { fn logs_dir(&self, bundle_id: impl Into<String>) -> Result<PathBuf, crate::Error> { - let base_dir = dirs::data_dir().unwrap(); + // SAFETY: In supported targets for the app, a data_dir is guaranteed by the OS; + // unwrap keeps this helper simple and avoids plumbing errors for a non-critical path. + let base_dir = dirs::data_dir().unwrap(); let logs_dir = base_dir.join(bundle_id.into()).join("logs"); let _ = std::fs::create_dir_all(&logs_dir); Ok(logs_dir) } }apps/desktop/src/components/settings/views/help-feedback.tsx (2)
20-24: Tiny cleanup: inline the then callback.Same behavior, less noise.
- const handleOpenLogs = () => { - tracingCommands.logsDir().then((logsDir) => { - openPath(logsDir); - }); - }; + const handleOpenLogs = () => { + tracingCommands.logsDir().then(openPath); + };
91-109: Use a distinct icon for Logs to avoid confusion with “Report a Bug”.Both entries use Bug; switch Logs to a document-style icon.
Apply within this block:
- <Bug className="h-5 w-5 text-gray-600" /> + <FileText className="h-5 w-5 text-gray-600" />Additionally (outside this range), update imports at the top:
import { Book, Bug, ExternalLinkIcon, MessageSquare, FileText } from "lucide-react";Optional: these buttons share identical structure; consider extracting a small <SettingsLinkButton …> to reduce duplication.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (5)
plugins/tracing/js/bindings.gen.tsis excluded by!**/*.gen.tsplugins/tracing/permissions/autogenerated/commands/logs_dir.tomlis excluded by!plugins/**/permissions/**plugins/tracing/permissions/autogenerated/reference.mdis excluded by!plugins/**/permissions/**plugins/tracing/permissions/default.tomlis excluded by!plugins/**/permissions/**plugins/tracing/permissions/schemas/schema.jsonis excluded by!plugins/**/permissions/**
📒 Files selected for processing (7)
apps/desktop/src/components/settings/views/help-feedback.tsx(2 hunks)crates/detect/src/list.rs(2 hunks)crates/whisper-local/src/model.rs(1 hunks)plugins/tracing/build.rs(1 hunks)plugins/tracing/src/commands.rs(1 hunks)plugins/tracing/src/ext.rs(1 hunks)plugins/tracing/src/lib.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- plugins/tracing/build.rs
- plugins/tracing/src/lib.rs
🧰 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:
crates/whisper-local/src/model.rsplugins/tracing/src/commands.rscrates/detect/src/list.rsapps/desktop/src/components/settings/views/help-feedback.tsxplugins/tracing/src/ext.rs
🧬 Code graph analysis (2)
plugins/tracing/src/commands.rs (1)
plugins/tracing/src/ext.rs (2)
logs_dir(4-4)logs_dir(8-13)
plugins/tracing/src/ext.rs (2)
plugins/tracing/src/commands.rs (1)
logs_dir(7-10)owhisper/owhisper-config/src/lib.rs (1)
data_dir(52-54)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ci (windows, windows-latest)
- GitHub Check: ci (macos, macos-latest)
🔇 Additional comments (5)
crates/detect/src/list.rs (2)
1-1: GatePathBufimport to avoid unused-imports on non-mac builds
With the functions now macOS-only,std::path::PathBufis unused on other targets and will fail builds if you deny warnings. Apply:- use std::path::PathBuf; +#[cfg(target_os = "macos")] +use std::path::PathBuf;Please confirm whether your crate has
#![deny(warnings)]or#![deny(unused_imports)]so this change is required.
10-45: Call sites are already gated; no cross-platform stub required.All uses of
list_installed_apps()inplugins/notification/src/ext.rsand the test incrates/detect/src/lib.rsare guarded with#[cfg(target_os)], so adding a non-mac stub isn’t necessary.apps/desktop/src/components/settings/views/help-feedback.tsx (3)
13-13: Old docs URL cleaned up — Verified no remaining references tohttps://hyprnote.com/docs.
5-6: Verified logsDir binding and plugin installation. tracingCommands.logsDir() is called in help-feedback.tsx (line 21), Rust’s logs_dir command exists in plugins/tracing/src/ext.rs and plugins/tracing/src/commands.rs, and @hypr/plugin-tracing is declared in apps/desktop/package.json.
2-2: plugin-opener capabilities confirmed
The default Tauri capabilities include opener:allow-open-url and opener:allow-open-path, so openUrl/openPath will work in production builds.
No description provided.