Warn instead of error for unparsable file names (#24775)

Skip instead of failing if we see unexpected file names in traces.

GitOrigin-RevId: 1571cfad3c99ac363e8ba065d3bc8958ce8a820a

Author: Preslav Le

crates/common/src/errors.rs

@@ -468,7 +468,7 @@ impl JsError {
         frame_data: Vec<FrameData>,
         custom_data: Option<ConvexValue>,
         mut lookup_source_map: impl FnMut(&ModuleSpecifier) -> anyhow::Result<Option<SourceMap>>,
-    ) -> anyhow::Result<Self> {
+    ) -> Self {
         let mut source_maps = BTreeMap::new();
         let mut mapped_frames = Vec::with_capacity(frame_data.len());
         for mut frame in frame_data {
@@ -479,10 +479,24 @@ impl JsError {
                 ..
             } = frame
             {
-                let specifier = ModuleSpecifier::parse(f)?;
+                let Ok(specifier) = ModuleSpecifier::parse(f) else {
+                    // We expect the file_name to be fully qualified URL but seems
+                    // this is not always the case. Lets log warning here.
+                    tracing::warn!("Skipping frame with invalid file_name: {f}");
+                    continue;
+                };
                 let source_map = match source_maps.entry(specifier) {
                     Entry::Vacant(e) => {
-                        let Some(source_map) = lookup_source_map(e.key())? else {
+                        let maybe_source_map = match lookup_source_map(e.key()) {
+                            Ok(maybe_source_map) => maybe_source_map,
+                            Err(err) => {
+                                // This is not expected so report an error.
+                                let mut err = err.context("Failed to lookup source_map");
+                                report_error(&mut err);
+                                continue;
+                            },
+                        };
+                        let Some(source_map) = maybe_source_map else {
                             tracing::debug!("Missing source map for {}", e.key());
                             continue;
                         };
@@ -518,15 +532,15 @@ impl JsError {
             mapped_frames.pop();
         }
 
-        Ok(JsError {
+        JsError {
             message,
             custom_data,
             frames: Some(JsFrames(mapped_frames.into())),
-        })
+        }
     }
 
     #[cfg(any(test, feature = "testing"))]
-    pub fn from_frames_for_test(message: &str, frames: Vec<&str>) -> anyhow::Result<Self> {
+    pub fn from_frames_for_test(message: &str, frames: Vec<&str>) -> Self {
         let frame_data = frames
             .into_iter()
             .map(|filename| FrameData {

crates/common/src/log_streaming.rs

@@ -107,7 +107,7 @@ impl LogEvent {
             cached: None,
         });
         Self::construct_exception(
-            &JsError::from_frames_for_test("test_message", vec!["test_frame_1", "test_frame_2"])?,
+            &JsError::from_frames_for_test("test_message", vec!["test_frame_1", "test_frame_2"]),
             runtime.unix_timestamp(),
             source,
             Some("1.5.1"),

crates/isolate/src/error.rs

@@ -49,9 +49,12 @@ impl<'a, 'b, RT: Runtime, E: IsolateEnvironment<RT>> ExecutionScope<'a, 'b, RT,
         exception: v8::Local<v8::Value>,
     ) -> anyhow::Result<JsError> {
         let (message, frame_data, custom_data) = extract_source_mapped_error(self, exception)?;
-        JsError::from_frames(message, frame_data, custom_data, |s| {
-            self.lookup_source_map(s)
-        })
+        Ok(JsError::from_frames(
+            message,
+            frame_data,
+            custom_data,
+            |s| self.lookup_source_map(s),
+        ))
     }
 
     pub fn lookup_source_map(

crates/isolate/src/isolate2/entered_context.rs

@@ -592,7 +592,7 @@ impl<'enter, 'scope: 'enter> EnteredContext<'enter, 'scope> {
                 extract_source_mapped_error(self.scope, exception)?;
             JsError::from_frames(message, frame_data, custom_data, |s| {
                 self.lookup_source_map(s)
-            })?
+            })
         };
         let err = match err {
             Ok(e) => e,

crates/isolate/src/ops/console.rs

@@ -27,7 +27,7 @@ pub fn op_console_trace<'b, P: OpProvider<'b>>(
 ) -> anyhow::Result<()> {
     let js_error = JsError::from_frames("".to_string(), frame_data, None, |s| {
         provider.lookup_source_map(s)
-    })?;
+    });
     messages.push(js_error.to_string());
     provider.trace(LogLevel::Log, messages)?;
     Ok(())

crates/isolate/src/ops/errors.rs

@@ -16,7 +16,7 @@ pub fn op_throw_uncatchable_developer_error<'b, P: OpProvider<'b>>(
 ) -> anyhow::Result<()> {
     let js_error = JsError::from_frames(message.clone(), frame_data, None, |s| {
         provider.lookup_source_map(s)
-    })?;
+    });
     report_error(&mut anyhow::anyhow!(format!(
         "UncatchableDeveloperError: {}",
         message
@@ -34,7 +34,7 @@ pub fn op_error_stack<'b, P: OpProvider<'b>>(
 ) -> anyhow::Result<String> {
     let js_error = JsError::from_frames(String::new(), frame_data, None, |s| {
         provider.lookup_source_map(s)
-    })?;
+    });
     Ok(js_error
         .frames
         .expect("JsError::from_frames has frames=None")

crates/node_executor/src/executor.rs

@@ -165,7 +165,7 @@ fn construct_js_error(
                 Ok(Some(sourcemap::SourceMap::from_slice(
                     source_map.as_bytes(),
                 )?))
-            })?
+            })
         },
         None => JsError::from_message(message),
     };
@@ -318,7 +318,7 @@ impl Actions {
                 if let Some(frames) = frames {
                     Ok(Err(JsError::from_frames(message, frames, None, |_| {
                         Ok(None)
-                    })?))
+                    })))
                 } else {
                     Ok(Err(JsError::from_message(message)))
                 }