mirror of
https://github.com/readest/readest.git
synced 2026-07-09 16:00:16 +00:00
fix(sentry): repair open-with crash-reporter arg and drop benign noise (#5014)
* fix(open-with): tolerate CLI arg parse failure from the crash reporter
sentry-minidump relaunches the app with `--crash-reporter-server`, which the
file-only tauri-plugin-cli schema rejects. parseCLIOpenWithFiles awaited
getMatches() and let the rejection surface as an unhandled rejection in the
webview (READEST-Y, 11 users, escalating). Catch it and degrade to "no CLI
files" so the open-with flow falls through to the intent path. Adds a
regression test.
Fixes READEST-Y
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(sentry): drop ResizeObserver noise and the contained mobi cover panic
Two before_send additions so known-benign events stop filling the backlog:
- "ResizeObserver loop limit exceeded" / "completed with undelivered
notifications" is a benign browser notice (layout still converges); add it to
is_ignored_browser_error (READEST-R, READEST-1Y, READEST-26).
- The mobi crate panics on a corrupt cover record; extract_cover already
catch_unwinds it so the import succeeds, but the panic hook still reports it.
Drop events whose stack hits mobi_parser::extract_cover, matched on our own
module path so unrelated slice panics are still reported (READEST-1Q,
READEST-10 follow-up).
Fixes READEST-R
Fixes READEST-1Y
Fixes READEST-26
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(sentry): drop the benign View-Transition timeout
The View-Transition timeout ("Transition was aborted because of timeout in DOM
update") fires when a nav's DOM update overruns the ~4s budget on a supported
engine (e.g. a large library grid on a slow device). The navigation still
completes without the animation, so it is not an app bug; it was 16 users of
unactionable backlog noise. Add it to is_ignored_browser_error alongside the
other benign View-Transition rejections. useAppRouter's API-support gate only
helps unsupported engines, and a client-side preventDefault would not stop
Sentry's forwarded capture, so dropping it in before_send is the reliable path.
Fixes READEST-9
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
5b8ab3db25
commit
fd82e5c2e4
4 changed files with 113 additions and 14 deletions
|
|
@ -332,7 +332,7 @@ pub fn run() {
|
|||
// version from the kernel string) so events group correctly.
|
||||
before_send: Some(std::sync::Arc::new(|mut event| {
|
||||
// Drop known-benign browser noise (e.g. View Transition
|
||||
// skipped/aborted) before it is reported.
|
||||
// skipped/aborted, ResizeObserver loop) before it is reported.
|
||||
if event.exception.values.iter().any(|ex| {
|
||||
ex.value
|
||||
.as_deref()
|
||||
|
|
@ -340,6 +340,21 @@ pub fn run() {
|
|||
}) {
|
||||
return None;
|
||||
}
|
||||
// Drop the contained MOBI cover panic: the `mobi` crate panics
|
||||
// on a corrupt cover record, which extract_cover catch_unwinds
|
||||
// (the import still succeeds), but the panic hook reports it
|
||||
// anyway. Match our own frame so unrelated slice panics stay.
|
||||
if event.exception.values.iter().any(|ex| {
|
||||
ex.stacktrace.iter().any(|st| {
|
||||
st.frames.iter().any(|f| {
|
||||
f.function
|
||||
.as_deref()
|
||||
.is_some_and(sentry_config::is_mobi_cover_panic_frame)
|
||||
})
|
||||
})
|
||||
}) {
|
||||
return None;
|
||||
}
|
||||
if let Some(sentry::protocol::Context::Os(os)) = event.contexts.get_mut("os") {
|
||||
if let Some(name) = sentry_config::corrected_os_name(
|
||||
std::env::consts::OS,
|
||||
|
|
|
|||
|
|
@ -77,17 +77,41 @@ pub fn android_version_from_uname(release: &str) -> Option<String> {
|
|||
}
|
||||
|
||||
/// Known-benign browser errors that are expected behavior, not app bugs, and
|
||||
/// only add noise to crash reporting: the View Transition API skips a transition
|
||||
/// when the tab is hidden or the navigation is superseded, and aborts it when the
|
||||
/// document is in an invalid state. These arrive as unhandled rejections while
|
||||
/// the navigation itself still completes. Matched (case-insensitively) on the
|
||||
/// exception value so they are dropped in `before_send`. NOTE: a transition
|
||||
/// *timeout* abort is deliberately NOT ignored — a slow DOM update can signal a
|
||||
/// real performance problem.
|
||||
/// only add noise to crash reporting:
|
||||
/// - The View Transition API skips a transition when the tab is hidden or the
|
||||
/// navigation is superseded, aborts it when the document is in an invalid
|
||||
/// state, and times out when the DOM update overruns its ~4s budget (e.g. a
|
||||
/// large library grid render on a slow device). All arrive as unhandled
|
||||
/// rejections while the navigation itself still completes without the
|
||||
/// animation, so none is an app bug (READEST-7 / READEST-F / READEST-G /
|
||||
/// READEST-9). The timeout was previously kept for its perf signal, but on
|
||||
/// supported engines it is just a slow render and only added backlog noise.
|
||||
/// - "ResizeObserver loop limit exceeded" / "ResizeObserver loop completed with
|
||||
/// undelivered notifications" is a benign notice the browser fires when
|
||||
/// observer callbacks don't settle within one frame; the spec defines it as
|
||||
/// safe to ignore and layout still converges (READEST-R / READEST-1Y /
|
||||
/// READEST-26).
|
||||
///
|
||||
/// Matched (case-insensitively) on the exception value so they are dropped in
|
||||
/// `before_send`.
|
||||
pub fn is_ignored_browser_error(value: &str) -> bool {
|
||||
let value = value.to_lowercase();
|
||||
value.contains("transition was skipped")
|
||||
|| value.contains("transition was aborted because of invalid state")
|
||||
|| value.contains("aborted because of timeout in dom update")
|
||||
|| value.contains("resizeobserver loop")
|
||||
}
|
||||
|
||||
/// Identifies a stack-frame function belonging to the MOBI cover extraction
|
||||
/// path. The third-party `mobi` crate panics on a corrupt cover record
|
||||
/// (an inverted slice range) when importing a truncated file (READEST-1Q /
|
||||
/// READEST-10). `mobi_parser::extract_cover` contains that panic with
|
||||
/// `catch_unwind` so the import still succeeds, but Sentry's panic hook reports
|
||||
/// it regardless; `before_send` drops events whose stack hits this frame so the
|
||||
/// contained panic stops adding noise. Matched on our own module path (not the
|
||||
/// generic slice-index message) so unrelated slice panics are still reported.
|
||||
pub fn is_mobi_cover_panic_frame(function: &str) -> bool {
|
||||
function.contains("mobi_parser::extract_cover")
|
||||
}
|
||||
|
||||
/// The WebView (engine, major-version), set once at startup when the app reports
|
||||
|
|
@ -151,7 +175,7 @@ pub extern "C" fn readest_sentry_dsn() -> *const std::os::raw::c_char {
|
|||
mod tests {
|
||||
use super::{
|
||||
android_version_from_uname, corrected_os_name, dsn_from_env, environment_for_version,
|
||||
is_ignored_browser_error, parse_webview_info, release_name,
|
||||
is_ignored_browser_error, is_mobi_cover_panic_frame, parse_webview_info, release_name,
|
||||
};
|
||||
|
||||
#[test]
|
||||
|
|
@ -234,14 +258,47 @@ mod tests {
|
|||
assert!(is_ignored_browser_error(
|
||||
"InvalidStateError: Transition was aborted because of invalid state"
|
||||
));
|
||||
// Timed out because a slow DOM update overran the budget (READEST-9).
|
||||
assert!(is_ignored_browser_error(
|
||||
"TimeoutError: Transition was aborted because of timeout in DOM update"
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn keeps_view_transition_timeout_and_real_errors() {
|
||||
// A transition timeout can signal a real perf problem — do NOT suppress it.
|
||||
assert!(!is_ignored_browser_error(
|
||||
"TimeoutError: Transition was aborted because of timeout in DOM update"
|
||||
fn matches_mobi_cover_panic_frame() {
|
||||
assert!(is_mobi_cover_panic_frame(
|
||||
"readestlib::mobi_parser::extract_cover"
|
||||
));
|
||||
// After catch_unwind the panicking frame is the inner fn.
|
||||
assert!(is_mobi_cover_panic_frame(
|
||||
"readestlib::mobi_parser::extract_cover_inner"
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn keeps_unrelated_panic_frames() {
|
||||
assert!(!is_mobi_cover_panic_frame(
|
||||
"readestlib::epub_parser::extract_epub_cover_full"
|
||||
));
|
||||
assert!(!is_mobi_cover_panic_frame(
|
||||
"core::slice::index::slice_index_fail"
|
||||
));
|
||||
assert!(!is_mobi_cover_panic_frame(""));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ignores_resize_observer_loop_noise() {
|
||||
// Both browser phrasings are benign (READEST-R / READEST-1Y / READEST-26).
|
||||
assert!(is_ignored_browser_error(
|
||||
"ResizeObserver loop limit exceeded"
|
||||
));
|
||||
assert!(is_ignored_browser_error(
|
||||
"Error: ResizeObserver loop completed with undelivered notifications."
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn keeps_real_errors() {
|
||||
assert!(!is_ignored_browser_error("TypeError: Load failed"));
|
||||
assert!(!is_ignored_browser_error("concurrent use forbidden"));
|
||||
assert!(!is_ignored_browser_error(""));
|
||||
|
|
|
|||
|
|
@ -151,6 +151,23 @@ describe('parseOpenWithFiles', () => {
|
|||
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
|
||||
test('degrades to intent when CLI arg parsing rejects (READEST-Y)', async () => {
|
||||
// sentry-minidump relaunches the app with `--crash-reporter-server`, which
|
||||
// the file-only CLI schema rejects. A rejected getMatches() must not leak
|
||||
// an unhandled rejection; fall through to the intent path instead.
|
||||
mockHasCli = true;
|
||||
mockGetMatches.mockRejectedValue(
|
||||
new Error(
|
||||
"failed to parse arguments: error: unexpected argument '--crash-reporter-server' found",
|
||||
),
|
||||
);
|
||||
mockGetCurrent.mockResolvedValue(null);
|
||||
|
||||
const result = await parseOpenWithFiles(null);
|
||||
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
// ── Intent / Deep Link ────────────────────────────────────────
|
||||
|
|
|
|||
|
|
@ -21,7 +21,17 @@ const parseWindowOpenWithFiles = () => {
|
|||
|
||||
const parseCLIOpenWithFiles = async () => {
|
||||
const { getMatches } = await import('@tauri-apps/plugin-cli');
|
||||
const matches = await getMatches();
|
||||
let matches;
|
||||
try {
|
||||
matches = await getMatches();
|
||||
} catch (err) {
|
||||
// getMatches() rejects when argv carries an option the file-only CLI schema
|
||||
// does not define. sentry-minidump relaunches the app with
|
||||
// `--crash-reporter-server`, which the parser rejects (READEST-Y). Treat a
|
||||
// parse failure as "no CLI files" instead of leaking an unhandled rejection.
|
||||
console.warn('Failed to parse CLI open-with args', err);
|
||||
return [];
|
||||
}
|
||||
const args = matches?.args;
|
||||
const files: string[] = [];
|
||||
if (args) {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue