Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
188 changes: 188 additions & 0 deletions app/src-tauri/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1142,6 +1142,25 @@ pub fn run() {
environment: Some(std::borrow::Cow::Owned(resolve_sentry_environment())),
send_default_pii: false,
before_send: Some(std::sync::Arc::new(|mut event| {
// Drop "dev-server fetch failed" noise: the vendored
// `tauri-runtime-cef` dev proxy
// (vendor/tauri-cef/crates/tauri/src/protocol/tauri.rs) calls
// `log::error!("Failed to request {url}: {err}")` whenever the
// CEF webview asks for an asset on `http://localhost:1420` (the
// Vite dev URL baked into `tauri.conf.json`). That `log::error!`
// is bridged into `tracing` and picked up by the sentry-tracing
// layer as an Event — see `src/core/logging.rs::sentry_tracing_layer`.
// In packaged staging/production builds Vite isn't running, so
// the request correctly fails — but the failure is noise we
// don't want in Sentry (issue OPENHUMAN-TAURI-V, 66+ events).
// See [sentry-localhost-filter] log line below for diagnostics.
if event_is_localhost_dev_fetch_noise(&event) {
log::debug!(
"[sentry-localhost-filter] dropping dev-server fetch noise event: {:?}",
event.message.as_deref().unwrap_or("<no message>")
);
return None;
}
// Strip server_name (hostname) to avoid leaking machine identity.
event.server_name = None;
event.user = None;
Expand Down Expand Up @@ -2159,6 +2178,56 @@ pub fn run_core_from_args(args: &[String]) -> Result<(), String> {
/// every surface (React frontend, core sidecar, Tauri shell) group under the
/// same release in Sentry and benefit from the same source-map / debug-info
/// upload.
/// Return `true` when the Sentry event is a "Failed to request
/// http://localhost:…" message originating from the vendored
/// `tauri-runtime-cef` dev-server proxy.
///
/// The proxy logs this message via `log::error!` (see
/// `app/src-tauri/vendor/tauri-cef/crates/tauri/src/protocol/tauri.rs`)
/// every time the CEF webview asks for an asset on the Vite dev URL
/// (`http://localhost:1420` per `tauri.conf.json`). In packaged
/// staging/production builds Vite isn't running, so the request fails —
/// but the failure is benign and shouldn't be reported.
///
/// The match is conservative: it checks the exact `Failed to request ` +
/// `http://localhost` / `http://127.0.0.1` prefix that only the dev-proxy
/// emits. Production HTTP errors from elsewhere in the shell or core use
/// different message shapes and won't be filtered.
fn event_is_localhost_dev_fetch_noise(event: &sentry::protocol::Event<'_>) -> bool {
// sentry-tracing 0.47 (with default `attach_stacktrace=false`) stores the
// log message in `event.message`. Check there first; fall back to the
// last exception's `value` for the (currently unused) stacktrace-enabled
// path so the filter stays correct if attach_stacktrace ever flips.
let direct = event.message.as_deref();
let from_exception = event.exception.last().and_then(|e| e.value.as_deref());
[direct, from_exception]
.into_iter()
.flatten()
.any(message_is_localhost_dev_fetch_noise)
}

/// Pure prefix check, separated from `event_is_localhost_dev_fetch_noise`
/// so the matching rule can be unit-tested without constructing a full
/// Sentry `Event`.
fn message_is_localhost_dev_fetch_noise(message: &str) -> bool {
// The tauri-cef dev proxy formats the message as:
// `Failed to request {url}: {err}`
// so anchoring on `Failed to request http://localhost` / `127.0.0.1` is
// sufficient and avoids matching unrelated "Failed to request …" errors
// elsewhere in the codebase that target real hosts.
//
// Note: no `[::1]` (IPv6 loopback) entry — the vendored tauri-cef dev
// proxy resolves `localhost` to IPv4 via reqwest's default resolver, so
// dev-server fetches always surface as `http://localhost:` or
// `http://127.0.0.1:`. Add an `[::1]` prefix if that ever changes
// (per graycyrus note on PR #1545).
const PREFIXES: &[&str] = &[
"Failed to request http://localhost:",
"Failed to request http://127.0.0.1:",
];
PREFIXES.iter().any(|p| message.starts_with(p))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[minor] No [::1] (IPv6 loopback) coverage. Likely fine since the vendored proxy resolves localhost to IPv4 via reqwest, but worth a one-line comment noting the omission so future readers dont wonder if it was forgotten.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 83ebf723 — added inline note explaining the absence of [::1] (tauri-cef dev proxy resolves localhost to IPv4 via reqwest default resolver, so dev-server fetches always surface as IPv4).

}

fn build_sentry_release_tag() -> String {
let version = env!("CARGO_PKG_VERSION");
let sha = option_env!("OPENHUMAN_BUILD_SHA").unwrap_or("").trim();
Expand Down Expand Up @@ -2527,4 +2596,123 @@ mod tests {
}
assert_eq!(env, "production");
}

// ── Sentry before_send filter: drop "Failed to request http://localhost:…"
// noise emitted by the vendored tauri-runtime-cef dev proxy in packaged
// builds (issue OPENHUMAN-TAURI-V). Tests target the pure
// `message_is_localhost_dev_fetch_noise` helper so the rule can be
// asserted without standing up a Sentry client.

#[test]
fn localhost_dev_fetch_noise_drops_vite_dev_url_1420() {
// The exact message shape reported by the latest event tag in Sentry
// (URL repeated by reqwest's `error sending request for url (…)`).
let msg = "Failed to request http://localhost:1420/components/skills/SkillCard.tsx: \
error sending request for url (http://localhost:1420/components/skills/SkillCard.tsx)";
assert!(
message_is_localhost_dev_fetch_noise(msg),
"expected Vite dev-server fetch failure to be filtered"
);
}

#[test]
fn localhost_dev_fetch_noise_drops_127_0_0_1_dev_url() {
// Some environments resolve `localhost` to 127.0.0.1 at the reqwest
// layer; the formatted message can carry either spelling.
let msg = "Failed to request http://127.0.0.1:1420/index.html: \
error sending request for url (http://127.0.0.1:1420/index.html)";
assert!(
message_is_localhost_dev_fetch_noise(msg),
"expected 127.0.0.1 dev-server fetch failure to be filtered"
);
}

#[test]
fn localhost_dev_fetch_noise_passes_production_url_through() {
// Real upstream failures (e.g. backend API errors surfaced via the
// same `Failed to request …` wording elsewhere) must NOT be filtered —
// they're the high-signal events Sentry exists for.
let msg = "Failed to request https://api.openhuman.ai/v1/skills: \
error sending request for url (https://api.openhuman.ai/v1/skills)";
assert!(
!message_is_localhost_dev_fetch_noise(msg),
"production API errors must NOT be filtered out"
);
}

#[test]
fn localhost_dev_fetch_noise_passes_unrelated_localhost_messages() {
// The filter is anchored on the dev-proxy's exact prefix to avoid
// accidentally dropping any error that happens to mention localhost
// (e.g. core-sidecar transport errors logged from coreRpcClient).
let msg =
"[core_rpc] transport error: error sending request for url (http://localhost:7788/rpc)";
assert!(
!message_is_localhost_dev_fetch_noise(msg),
"non-tauri-cef localhost errors must NOT be filtered"
);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Consider adding 1-2 event-level tests for event_is_localhost_dev_fetch_noise to cover the event.message vs exception.last().value fallback path. Not blocking since the wiring is trivial, but would catch regressions if sentry-tracing changes how it populates events.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 83ebf723 — added 3 event-level tests: event_filter_uses_message_field (primary path), event_filter_falls_back_to_last_exception_value (fallback path), event_filter_passes_through_when_neither_field_matches (negative case). 8/8 filter tests pass.


#[test]
fn event_filter_uses_message_field() {
// event-level coverage: when sentry-tracing populates
// `event.message` (default with `attach_stacktrace=false`), the
// filter should see the noise payload through the primary read
// path. Per graycyrus on PR #1545.
let mut event = sentry::protocol::Event::new();
event.message = Some("Failed to request http://localhost:1420/foo: timeout".into());
assert!(
event_is_localhost_dev_fetch_noise(&event),
"event.message read path must catch noise messages"
);
}

#[test]
fn event_filter_falls_back_to_last_exception_value() {
// event-level coverage: if `attach_stacktrace` is ever turned on,
// sentry-tracing populates `event.exception` instead of (or in
// addition to) `event.message`. Filter must still see the noise
// payload through the exception fallback. Per graycyrus on PR #1545.
let mut event = sentry::protocol::Event::new();
event.message = None;
event.exception.values.push(sentry::protocol::Exception {
ty: "log".into(),
value: Some("Failed to request http://localhost:1420/foo: timeout".into()),
..Default::default()
});
assert!(
event_is_localhost_dev_fetch_noise(&event),
"exception fallback must catch noise messages when event.message is absent"
);
}

#[test]
fn event_filter_passes_through_when_neither_field_matches() {
// Negative event-level case: no noise prefix in either field →
// event must NOT be filtered.
let mut event = sentry::protocol::Event::new();
event.message = Some("genuine production error".into());
event.exception.values.push(sentry::protocol::Exception {
ty: "log".into(),
value: Some("connection refused (10061)".into()),
..Default::default()
});
assert!(
!event_is_localhost_dev_fetch_noise(&event),
"legitimate production events must pass through"
);
}

#[test]
fn localhost_dev_fetch_noise_anchors_to_message_start() {
// CodeRabbit (PR #1545) caught that the predicate used
// `contains` rather than `starts_with`. Regression: a message
// that merely embeds the dev-proxy prefix later in its text
// must NOT be filtered — only messages that *begin* with it.
let msg = "User report: `Failed to request http://localhost:1420/foo` was logged earlier";
assert!(
!message_is_localhost_dev_fetch_noise(msg),
"messages that merely contain the dev-proxy prefix must NOT be filtered"
);
}
}
Loading