From 78ff1a20f26d998fec598eddabb7cac26ef97634 Mon Sep 17 00:00:00 2001 From: Sebastian Zivota Date: Thu, 18 Apr 2024 16:17:37 +0200 Subject: [PATCH] fix: Always trim source context lines (#1443) --- crates/symbolicator-js/src/symbolication.rs | 2 +- .../src/symbolication/module_lookup.rs | 2 +- .../src/symbolication.rs | 2 +- .../src/source_context.rs | 50 ++++++++++++++----- 4 files changed, 40 insertions(+), 16 deletions(-) diff --git a/crates/symbolicator-js/src/symbolication.rs b/crates/symbolicator-js/src/symbolication.rs index ad9e690ff..b2bfae434 100644 --- a/crates/symbolicator-js/src/symbolication.rs +++ b/crates/symbolicator-js/src/symbolication.rs @@ -290,7 +290,7 @@ async fn symbolicate_js_frame( fn apply_source_context(frame: &mut JsFrame, source: &str) -> Result<(), JsModuleErrorKind> { let lineno = frame.lineno as usize; - let column = frame.colno.map(|col| col as usize); + let column = frame.colno.map(|col| col as usize).unwrap_or_default(); if let Some((pre_context, context_line, post_context)) = get_context_lines(source, lineno, column, None) diff --git a/crates/symbolicator-native/src/symbolication/module_lookup.rs b/crates/symbolicator-native/src/symbolication/module_lookup.rs index a57820470..3c8842204 100644 --- a/crates/symbolicator-native/src/symbolication/module_lookup.rs +++ b/crates/symbolicator-native/src/symbolication/module_lookup.rs @@ -431,7 +431,7 @@ impl ModuleLookup { pub(crate) fn set_source_context(source: &str, frame: &mut RawFrame) -> Option<()> { let (pre_context, context_line, post_context) = - get_context_lines(source, frame.lineno?.try_into().ok()?, None, None)?; + get_context_lines(source, frame.lineno?.try_into().ok()?, 0, None)?; frame.pre_context = pre_context; frame.context_line = Some(context_line); frame.post_context = post_context; diff --git a/crates/symbolicator-proguard/src/symbolication.rs b/crates/symbolicator-proguard/src/symbolication.rs index bba51af72..e784812d2 100644 --- a/crates/symbolicator-proguard/src/symbolication.rs +++ b/crates/symbolicator-proguard/src/symbolication.rs @@ -367,7 +367,7 @@ impl ProguardService { }; if let Some((pre_context, context_line, post_context)) = - get_context_lines(contents, lineno as usize, None, None) + get_context_lines(contents, lineno as usize, 0, None) { frame.pre_context = pre_context; frame.context_line = Some(context_line); diff --git a/crates/symbolicator-service/src/source_context.rs b/crates/symbolicator-service/src/source_context.rs index 79ea1366f..d87256e78 100644 --- a/crates/symbolicator-service/src/source_context.rs +++ b/crates/symbolicator-service/src/source_context.rs @@ -4,14 +4,15 @@ pub const DEFAULT_CONTEXT_LINES: usize = 5; /// /// N (`context_lines`) lines of context will be resolved in addition to `lineno`. /// This defaults to [`DEFAULT_CONTEXT_LINES`]. -/// If `trim_to_column` is provided, every output line will be truncated to ~150 characters -/// centered around the provided column number, and a `{snip}` marker is added at the trimmed edges. +/// Every output line will be truncated to ~150 characters, centered as best as possible +/// around `trim_to_column`, and a `{snip}` marker is added at the trimmed edges. +/// To simply cut off lines after ~150 characters, pass 0. /// /// If no source line for `lineno` is found in `source`, it will return [`None`] instead. pub fn get_context_lines( source: &str, lineno: usize, - trim_to_column: Option, + trim_to_column: usize, context_lines: Option, ) -> Option<(Vec, String, Vec)> { let context_lines = context_lines.unwrap_or(DEFAULT_CONTEXT_LINES); @@ -19,18 +20,12 @@ pub fn get_context_lines( let start_line = lineno.saturating_sub(context_lines).saturating_sub(1); let line_diff = (lineno - start_line).saturating_sub(1); - let maybe_trim_line = |line: &str| { - if let Some(column) = trim_to_column { - trim_context_line(line, column) - } else { - line.to_string() - } - }; + let trim_line = |line: &str| trim_context_line(line, trim_to_column); let mut lines = source.lines().skip(start_line); - let pre_context = (&mut lines).take(line_diff).map(maybe_trim_line).collect(); - let context = maybe_trim_line(lines.next()?); - let post_context = lines.take(context_lines).map(maybe_trim_line).collect(); + let pre_context = (&mut lines).take(line_diff).map(trim_line).collect(); + let context = trim_line(lines.next()?); + let post_context = lines.take(context_lines).map(trim_line).collect(); Some((pre_context, context, post_context)) } @@ -120,4 +115,33 @@ mod tests { "{snip} ๐Ÿงก๐Ÿ’›๐Ÿ’š๐Ÿ’™๐Ÿ’œ.โค๏ธ๐Ÿงก๐Ÿ’›๐Ÿ’š๐Ÿ’™๐Ÿ’œ.โค๏ธ๐Ÿงก๐Ÿ’›๐Ÿ’š๐Ÿ’™๐Ÿ’œ.โค๏ธ๐Ÿงก๐Ÿ’›๐Ÿ’š๐Ÿ’™๐Ÿ’œ.โค๏ธ๐Ÿงก๐Ÿ’›๐Ÿ’š๐Ÿ’™๐Ÿ’œ.โค๏ธ๐Ÿงก๐Ÿ’› {snip}" ); } + + #[test] + fn test_get_context_lines() { + let source = "\ +Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Sit amet mattis vulputate enim nulla aliquet porttitor. Dis parturient montes nascetur ridiculus mus mauris vitae ultricies. Eget lorem dolor sed viverra ipsum. Tempor id eu nisl nunc mi ipsum faucibus vitae aliquet. Aliquam etiam erat velit scelerisque in dictum non. Blandit aliquam etiam erat velit. Maecenas sed enim ut sem viverra. Facilisi nullam vehicula ipsum a arcu cursus vitae. Non enim praesent elementum facilisis leo vel. Egestas congue quisque egestas diam in arcu cursus euismod quis. Malesuada fames ac turpis egestas maecenas. Non quam lacus suspendisse faucibus interdum posuere lorem ipsum. Felis eget nunc lobortis mattis aliquam faucibus purus in massa. Posuere morbi leo urna molestie at elementum eu facilisis sed. Cras ornare arcu dui vivamus arcu felis bibendum ut. Nibh tortor id aliquet lectus proin nibh nisl condimentum id. + +Netus et malesuada fames ac turpis egestas. Id eu nisl nunc mi ipsum faucibus vitae aliquet nec. Adipiscing commodo elit at imperdiet dui accumsan sit amet. Dui id ornare arcu odio ut sem. In nisl nisi scelerisque eu. Consequat semper viverra nam libero. Lacus laoreet non curabitur gravida arcu ac tortor dignissim convallis. At augue eget arcu dictum. Ac turpis egestas maecenas pharetra. Nisi est sit amet facilisis magna etiam tempor orci. Eget arcu dictum varius duis. Nunc lobortis mattis aliquam faucibus purus in massa tempor. Id leo in vitae turpis. Et malesuada fames ac turpis egestas. Dictum fusce ut placerat orci nulla pellentesque. Cras pulvinar mattis nunc sed blandit libero volutpat sed cras. + +Consequat ac felis donec et odio pellentesque diam volutpat commodo. Pulvinar elementum integer enim neque volutpat ac. Mollis aliquam ut porttitor leo a diam sollicitudin. Velit ut tortor pretium viverra suspendisse potenti nullam. Viverra tellus in hac habitasse platea dictumst vestibulum rhoncus. Dictum non consectetur a erat nam at lectus urna duis. Condimentum vitae sapien pellentesque habitant morbi tristique senectus et. Tempus urna et pharetra pharetra massa massa ultricies. Integer malesuada nunc vel risus commodo viverra maecenas. Neque viverra justo nec ultrices dui sapien. Fermentum leo vel orci porta non. A diam maecenas sed enim ut sem viverra aliquet eget. Tincidunt ornare massa eget egestas purus. Curabitur gravida arcu ac tortor dignissim convallis aenean. +"; + + // trim to column 0 + let (before, line, after) = get_context_lines(source, 3, 0, Some(2)).unwrap(); + assert_eq!(before, vec!["Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Sit amet mattis {snip}", ""]); + assert_eq!(line, "Netus et malesuada fames ac turpis egestas. Id eu nisl nunc mi ipsum faucibus vitae aliquet nec. Adipiscing commodo elit at imperdiet dui ac {snip}"); + assert_eq!(after, vec!["", "Consequat ac felis donec et odio pellentesque diam volutpat commodo. Pulvinar elementum integer enim neque volutpat ac. Mollis aliquam ut po {snip}"]); + + // trim to column 500 + let (before, line, after) = get_context_lines(source, 3, 500, Some(2)).unwrap(); + assert_eq!(before, vec!["{snip} enim ut sem viverra. Facilisi nullam vehicula ipsum a arcu cursus vitae. Non enim praesent elementum facilisis leo vel. Egestas congue quisq {snip}", ""]); + assert_eq!(line, "{snip} ci. Eget arcu dictum varius duis. Nunc lobortis mattis aliquam faucibus purus in massa tempor. Id leo in vitae turpis. Et malesuada fames ac {snip}"); + assert_eq!(after, vec!["", "{snip} rna et pharetra pharetra massa massa ultricies. Integer malesuada nunc vel risus commodo viverra maecenas. Neque viverra justo nec ultrices {snip}"]); + + // trim to column 1000 + let (before, line, after) = get_context_lines(source, 3, 1000, Some(2)).unwrap(); + assert_eq!(before, vec!["{snip} ementum eu facilisis sed. Cras ornare arcu dui vivamus arcu felis bibendum ut. Nibh tortor id aliquet lectus proin nibh nisl condimentum id.", ""]); + assert_eq!(line, "{snip} a fames ac turpis egestas. Dictum fusce ut placerat orci nulla pellentesque. Cras pulvinar mattis nunc sed blandit libero volutpat sed cras."); + assert_eq!(after, vec!["", "{snip} ed enim ut sem viverra aliquet eget. Tincidunt ornare massa eget egestas purus. Curabitur gravida arcu ac tortor dignissim convallis aenean."]); + } }