diff --git a/crates/napi/src/lib.rs b/crates/napi/src/lib.rs index 0fe70803a5521..240fe9077c1b7 100644 --- a/crates/napi/src/lib.rs +++ b/crates/napi/src/lib.rs @@ -68,11 +68,19 @@ static ALLOC: dhat::Alloc = dhat::Alloc; #[cfg(not(target_arch = "wasm32"))] #[napi::module_init] - fn init() { + use std::panic::{set_hook, take_hook}; + use tokio::runtime::Builder; + use turbo_tasks::handle_panic; use turbo_tasks_malloc::TurboMalloc; + let prev_hook = take_hook(); + set_hook(Box::new(move |info| { + handle_panic(info); + prev_hook(info); + })); + let rt = Builder::new_multi_thread() .enable_all() .on_thread_stop(|| { diff --git a/turbopack/crates/turbo-tasks-backend/src/backend/mod.rs b/turbopack/crates/turbo-tasks-backend/src/backend/mod.rs index 001bb6ee78cb3..33a2e75d4ac8d 100644 --- a/turbopack/crates/turbo-tasks-backend/src/backend/mod.rs +++ b/turbopack/crates/turbo-tasks-backend/src/backend/mod.rs @@ -28,7 +28,7 @@ use turbo_tasks::{ SessionId, TRANSIENT_TASK_BIT, TaskId, TraitTypeId, TurboTasksBackendApi, ValueTypeId, backend::{ Backend, BackendJobId, CachedTaskType, CellContent, TaskExecutionSpec, TransientTaskRoot, - TransientTaskType, TypedCellContent, + TransientTaskType, TurboTasksExecutionError, TypedCellContent, }, event::{Event, EventListener}, registry::{self, get_value_type_global_name}, @@ -553,9 +553,7 @@ impl TurboTasksBackendInner { let result = match output { OutputValue::Cell(cell) => Some(Ok(Ok(RawVc::TaskCell(cell.task, cell.cell)))), OutputValue::Output(task) => Some(Ok(Ok(RawVc::TaskOutput(*task)))), - OutputValue::Error | OutputValue::Panic => { - get!(task, Error).map(|error| Err(error.clone().into())) - } + OutputValue::Error => get!(task, Error).map(|error| Err(error.clone().into())), }; if let Some(result) = result { if self.should_track_dependencies() { @@ -1406,7 +1404,7 @@ impl TurboTasksBackendInner { fn task_execution_result( &self, task_id: TaskId, - result: Result, Option>>, + result: Result>, turbo_tasks: &dyn TurboTasksBackendApi>, ) { operation::UpdateOutputOperation::run(task_id, result, self.execute_context(turbo_tasks)); @@ -2338,7 +2336,7 @@ impl Backend for TurboTasksBackend { fn task_execution_result( &self, task_id: TaskId, - result: Result, Option>>, + result: Result>, turbo_tasks: &dyn TurboTasksBackendApi, ) { self.0.task_execution_result(task_id, result, turbo_tasks); diff --git a/turbopack/crates/turbo-tasks-backend/src/backend/operation/update_output.rs b/turbopack/crates/turbo-tasks-backend/src/backend/operation/update_output.rs index 1e563d17830c7..92fe8b62211d6 100644 --- a/turbopack/crates/turbo-tasks-backend/src/backend/operation/update_output.rs +++ b/turbopack/crates/turbo-tasks-backend/src/backend/operation/update_output.rs @@ -1,8 +1,8 @@ -use std::{borrow::Cow, mem::take}; +use std::{mem::take, sync::Arc}; -use anyhow::{Result, anyhow}; +use anyhow::Result; use serde::{Deserialize, Serialize}; -use turbo_tasks::{RawVc, TaskId, util::SharedError}; +use turbo_tasks::{RawVc, TaskId, backend::TurboTasksExecutionError, util::SharedError}; #[cfg(feature = "trace_task_dirty")] use crate::backend::operation::invalidate::TaskDirtyCause; @@ -44,7 +44,7 @@ pub enum UpdateOutputOperation { impl UpdateOutputOperation { pub fn run( task_id: TaskId, - output: Result, Option>>, + output: Result>, mut ctx: impl ExecuteContext, ) { let mut task = ctx.task(task_id, TaskDataCategory::All); @@ -69,7 +69,7 @@ impl UpdateOutputOperation { let old_error = task.remove(&CachedDataItemKey::Error {}); let current_output = get!(task, Output); let output_value = match output { - Ok(Ok(RawVc::TaskOutput(output_task_id))) => { + Ok(RawVc::TaskOutput(output_task_id)) => { if let Some(OutputValue::Output(current_task_id)) = current_output { if *current_task_id == output_task_id { return; @@ -77,7 +77,7 @@ impl UpdateOutputOperation { } OutputValue::Output(output_task_id) } - Ok(Ok(RawVc::TaskCell(output_task_id, cell))) => { + Ok(RawVc::TaskCell(output_task_id, cell)) => { if let Some(OutputValue::Cell(CellRef { task: current_task_id, cell: current_cell, @@ -92,28 +92,18 @@ impl UpdateOutputOperation { cell, }) } - Ok(Ok(RawVc::LocalOutput(..))) => { + Ok(RawVc::LocalOutput(..)) => { panic!("Non-local tasks must not return a local Vc"); } - Ok(Err(err)) => { + Err(err) => { task.insert(CachedDataItem::Error { - value: SharedError::new(err.context(format!( + value: SharedError::new(anyhow::Error::new(err).context(format!( "Execution of {} failed", ctx.get_task_description(task_id) ))), }); OutputValue::Error } - Err(panic) => { - task.insert(CachedDataItem::Error { - value: SharedError::new(anyhow!( - "Panic in {}: {:?}", - ctx.get_task_description(task_id), - panic - )), - }); - OutputValue::Panic - } }; let old_content = task.insert(CachedDataItem::Output { value: output_value, diff --git a/turbopack/crates/turbo-tasks-backend/src/data.rs b/turbopack/crates/turbo-tasks-backend/src/data.rs index 942e8002cc83b..7c6e40426484d 100644 --- a/turbopack/crates/turbo-tasks-backend/src/data.rs +++ b/turbopack/crates/turbo-tasks-backend/src/data.rs @@ -57,7 +57,6 @@ pub enum OutputValue { Cell(CellRef), Output(TaskId), Error, - Panic, } impl OutputValue { fn is_transient(&self) -> bool { @@ -65,7 +64,6 @@ impl OutputValue { OutputValue::Cell(cell) => cell.task.is_transient(), OutputValue::Output(task) => task.is_transient(), OutputValue::Error => false, - OutputValue::Panic => false, } } } diff --git a/turbopack/crates/turbo-tasks-backend/tests/panics.rs b/turbopack/crates/turbo-tasks-backend/tests/panics.rs new file mode 100644 index 0000000000000..3b934eecd59a3 --- /dev/null +++ b/turbopack/crates/turbo-tasks-backend/tests/panics.rs @@ -0,0 +1,49 @@ +use core::panic; +use std::{ + panic::{set_hook, take_hook}, + sync::{Arc, LazyLock}, +}; + +use anyhow::Result; +use regex::Regex; +use turbo_tasks::{Vc, backend::TurboTasksExecutionError, handle_panic}; +use turbo_tasks_testing::{Registration, register, run_without_cache_check}; + +static REGISTRATION: Registration = register!(); + +static FILE_PATH_REGEX: LazyLock = + LazyLock::new(|| Regex::new(r"panics\.rs:\d+:\d+$").unwrap()); + +#[tokio::test] +async fn test_panics_include_location() { + let prev_hook = take_hook(); + set_hook(Box::new(move |info| { + handle_panic(info); + prev_hook(info); + })); + + let result = + run_without_cache_check(®ISTRATION, async move { anyhow::Ok(*double(3).await?) }).await; + + let error = result.unwrap_err(); + let root_cause = error.root_cause(); + + let Some(panic) = root_cause.downcast_ref::>() else { + panic!("Expected a TurboTasksExecutionError"); + }; + + let TurboTasksExecutionError::Panic(panic) = &**panic else { + panic!("Expected a TurboTasksExecutionError::Panic"); + }; + + assert!( + FILE_PATH_REGEX.is_match(panic.location.as_ref().unwrap()), + "Panic location '{}' should be a line in panics.rs", + panic.location.as_ref().unwrap() + ); +} + +#[turbo_tasks::function] +fn double(_val: u64) -> Result> { + panic!("oh no"); +} diff --git a/turbopack/crates/turbo-tasks-backend/tests/trace_transient.rs b/turbopack/crates/turbo-tasks-backend/tests/trace_transient.rs index f68012ba07a2e..74c21fcaebb65 100644 --- a/turbopack/crates/turbo-tasks-backend/tests/trace_transient.rs +++ b/turbopack/crates/turbo-tasks-backend/tests/trace_transient.rs @@ -32,12 +32,9 @@ async fn test_trace_transient() { anyhow::Ok(()) }) .await; - assert!( - result - .unwrap_err() - .to_string() - .contains(&EXPECTED_TRACE.escape_debug().to_string()) - ); + + let message = format!("{:#}", result.unwrap_err()); + assert!(message.contains(&EXPECTED_TRACE.to_string())); } #[turbo_tasks::value] diff --git a/turbopack/crates/turbo-tasks-backend/tests/transient_collectible.rs b/turbopack/crates/turbo-tasks-backend/tests/transient_collectible.rs index 5610dfa7686b4..eabd554106503 100644 --- a/turbopack/crates/turbo-tasks-backend/tests/transient_collectible.rs +++ b/turbopack/crates/turbo-tasks-backend/tests/transient_collectible.rs @@ -19,12 +19,9 @@ async fn test_transient_emit_from_persistent() { anyhow::Ok(()) }) .await; - assert!( - result - .unwrap_err() - .to_string() - .contains(&EXPECTED_MSG.escape_debug().to_string()) - ); + + let message = format!("{:#}", result.unwrap_err()); + assert!(message.contains(&EXPECTED_MSG.to_string())); } #[turbo_tasks::function(operation)] diff --git a/turbopack/crates/turbo-tasks-memory/src/memory_backend.rs b/turbopack/crates/turbo-tasks-memory/src/memory_backend.rs index 4db5004a9ec60..4fcd2d6ce47b5 100644 --- a/turbopack/crates/turbo-tasks-memory/src/memory_backend.rs +++ b/turbopack/crates/turbo-tasks-memory/src/memory_backend.rs @@ -1,5 +1,5 @@ use std::{ - borrow::{Borrow, Cow}, + borrow::Borrow, future::Future, hash::{BuildHasher, BuildHasherDefault, Hash}, num::NonZeroU32, @@ -22,7 +22,7 @@ use turbo_tasks::{ TaskIdSet, TraitTypeId, TurboTasksBackendApi, Unused, ValueTypeId, backend::{ Backend, BackendJobId, CachedTaskType, CellContent, TaskCollectiblesMap, TaskExecutionSpec, - TransientTaskType, TypedCellContent, + TransientTaskType, TurboTasksExecutionError, TypedCellContent, }, event::EventListener, task_statistics::TaskStatisticsApi, @@ -415,12 +415,12 @@ impl Backend for MemoryBackend { fn task_execution_result( &self, task_id: TaskId, - result: Result, Option>>, + result: Result>, turbo_tasks: &dyn TurboTasksBackendApi, ) { self.with_task(task_id, |task| { #[cfg(debug_assertions)] - if let Ok(Ok(RawVc::TaskOutput(result))) = result.as_ref() { + if let Ok(RawVc::TaskOutput(result)) = result.as_ref() { if *result == task_id { panic!("Task {} returned itself as output", task.get_description()); } diff --git a/turbopack/crates/turbo-tasks-memory/src/output.rs b/turbopack/crates/turbo-tasks-memory/src/output.rs index 47fd63eb2b330..94d2fa14ea8ff 100644 --- a/turbopack/crates/turbo-tasks-memory/src/output.rs +++ b/turbopack/crates/turbo-tasks-memory/src/output.rs @@ -1,4 +1,4 @@ -use std::{borrow::Cow, fmt::Debug, mem::take}; +use std::{fmt::Debug, mem::take}; use anyhow::{Error, Result, anyhow}; use turbo_tasks::{ @@ -51,18 +51,6 @@ impl Output { } } - pub fn panic( - &mut self, - message: Option>, - turbo_tasks: &dyn TurboTasksBackendApi, - ) { - self.content = Some(OutputContent::Panic(message.map(Box::new))); - // notify - if !self.dependent_tasks.is_empty() { - turbo_tasks.schedule_notify_tasks_set(&take(&mut self.dependent_tasks)); - } - } - pub fn gc_drop(self, turbo_tasks: &dyn TurboTasksBackendApi) { // notify if !self.dependent_tasks.is_empty() { diff --git a/turbopack/crates/turbo-tasks-memory/src/task.rs b/turbopack/crates/turbo-tasks-memory/src/task.rs index ede71cf441b75..8a861c3d5ce8f 100644 --- a/turbopack/crates/turbo-tasks-memory/src/task.rs +++ b/turbopack/crates/turbo-tasks-memory/src/task.rs @@ -1,5 +1,4 @@ use std::{ - borrow::Cow, fmt::{self, Debug, Display, Formatter}, future::Future, hash::{BuildHasherDefault, Hash}, @@ -21,7 +20,10 @@ use turbo_prehash::PreHashed; use turbo_tasks::{ CellId, Invalidator, RawVc, ReadConsistency, TaskId, TaskIdSet, TraitTypeId, TurboTasksBackendApi, TurboTasksBackendApiExt, ValueTypeId, - backend::{CachedTaskType, CellContent, TaskCollectiblesMap, TaskExecutionSpec}, + backend::{ + CachedTaskType, CellContent, TaskCollectiblesMap, TaskExecutionSpec, + TurboTasksExecutionError, + }, event::{Event, EventListener}, get_invalidator, registry, }; @@ -837,7 +839,7 @@ impl Task { pub(crate) fn execution_result( &self, - result: Result, Option>>, + result: Result>, backend: &MemoryBackend, turbo_tasks: &dyn TurboTasksBackendApi, ) { @@ -849,7 +851,7 @@ impl Task { // TODO maybe this should be controlled by a heuristic } InProgress(..) => match result { - Ok(Ok(result)) => { + Ok(result) => { if state.output != result { if backend.print_task_invalidation && state.output.content.is_some() { println!( @@ -865,13 +867,17 @@ impl Task { state.output.link(result, turbo_tasks) } } - Ok(Err(mut err)) => { - if let Some(name) = self.get_function_name() { - err = err.context(format!("Execution of {name} failed")); - } + Err(err) => { + let err = anyhow::Error::new(err).context( + if let Some(name) = self.get_function_name() { + format!("Execution of {name} failed") + } else { + "Execution failed".to_string() + }, + ); + state.output.error(err, turbo_tasks) } - Err(message) => state.output.panic(message, turbo_tasks), }, Dirty { .. } | Scheduled { .. } | Done { .. } => { diff --git a/turbopack/crates/turbo-tasks/src/backend.rs b/turbopack/crates/turbo-tasks/src/backend.rs index 5a573d78c4082..dbe2d8ab0711f 100644 --- a/turbopack/crates/turbo-tasks/src/backend.rs +++ b/turbopack/crates/turbo-tasks/src/backend.rs @@ -1,9 +1,10 @@ use std::{ - borrow::Cow, + error::Error, fmt::{self, Debug, Display}, future::Future, hash::{BuildHasherDefault, Hash}, pin::Pin, + sync::Arc, time::Duration, }; @@ -15,7 +16,7 @@ use tracing::Span; pub use crate::id::BackendJobId; use crate::{ FunctionId, RawVc, ReadCellOptions, ReadRef, SharedReference, TaskId, TaskIdSet, TraitRef, - TraitTypeId, ValueTypeId, VcRead, VcValueTrait, VcValueType, + TraitTypeId, TurboTasksPanic, ValueTypeId, VcRead, VcValueTrait, VcValueType, event::EventListener, magic_any::MagicAny, manager::{ReadConsistency, TurboTasksBackendApi}, @@ -398,6 +399,80 @@ impl TryFrom for SharedReference { pub type TaskCollectiblesMap = AutoMap, 1>; +// Structurally and functionally similar to Cow<&'static, str> but explicitly notes the importance +// of non-static strings potentially containing PII. +#[derive(Clone, Debug)] +pub enum TurboTasksExecutionErrorMessage { + PIISafe(&'static str), + NonPIISafe(String), +} + +impl Display for TurboTasksExecutionErrorMessage { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + TurboTasksExecutionErrorMessage::PIISafe(msg) => write!(f, "{msg}"), + TurboTasksExecutionErrorMessage::NonPIISafe(msg) => write!(f, "{msg}"), + } + } +} + +#[derive(Clone, Debug)] +pub enum TurboTasksExecutionError { + Panic(Arc), + Error { + message: TurboTasksExecutionErrorMessage, + source: Option>, + }, +} + +impl Error for TurboTasksExecutionError { + fn source(&self) -> Option<&(dyn Error + 'static)> { + match self { + TurboTasksExecutionError::Panic(_panic) => None, + TurboTasksExecutionError::Error { source, .. } => { + source.as_ref().map(|s| s as &dyn Error) + } + } + } +} + +impl Display for TurboTasksExecutionError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + TurboTasksExecutionError::Panic(panic) => write!(f, "{}", &panic), + TurboTasksExecutionError::Error { message, .. } => { + write!(f, "{message}") + } + } + } +} + +impl From for TurboTasksExecutionError { + fn from(err: anyhow::Error) -> Self { + let mut current: &dyn std::error::Error = err.as_ref(); + let mut message = current.to_string(); + let mut source_exec_error = None; + + // Flatten non-TurboTasksExecutionErrors in the error chain into a single message. + // Once a TurboTasksExecutionError is found, use that as the source. + while let Some(current_source) = current.source() { + if let Some(turbo_error) = + current_source.downcast_ref::>() + { + source_exec_error = Some(turbo_error.clone()); + break; + } + message.push_str(&format!("\n{current_source}")); + current = current_source; + } + + TurboTasksExecutionError::Error { + message: TurboTasksExecutionErrorMessage::NonPIISafe(message), + source: source_exec_error, + } + } +} + pub trait Backend: Sync + Send { #[allow(unused_variables)] fn startup(&self, turbo_tasks: &dyn TurboTasksBackendApi) {} @@ -459,8 +534,8 @@ pub trait Backend: Sync + Send { fn task_execution_result( &self, - task: TaskId, - result: Result, Option>>, + task_id: TaskId, + result: Result>, turbo_tasks: &dyn TurboTasksBackendApi, ); diff --git a/turbopack/crates/turbo-tasks/src/capture_future.rs b/turbopack/crates/turbo-tasks/src/capture_future.rs index 8b5d2b21b6e27..e4996d7915845 100644 --- a/turbopack/crates/turbo-tasks/src/capture_future.rs +++ b/turbopack/crates/turbo-tasks/src/capture_future.rs @@ -1,14 +1,19 @@ use std::{ cell::RefCell, + fmt::Display, future::Future, + panic, pin::Pin, task::{Context, Poll}, time::{Duration, Instant}, }; +use anyhow::Result; use pin_project_lite::pin_project; use turbo_tasks_malloc::{AllocationInfo, TurboMalloc}; +use crate::{LAST_ERROR_LOCATION, backend::TurboTasksExecutionErrorMessage}; + struct ThreadLocalData { duration: Duration, allocations: usize, @@ -64,8 +69,20 @@ pub fn add_allocation_info(alloc_info: AllocationInfo) { }); } +#[derive(Debug, Clone)] +pub struct TurboTasksPanic { + pub message: TurboTasksExecutionErrorMessage, + pub location: Option, +} + +impl Display for TurboTasksPanic { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.message) + } +} + impl> Future for CaptureFuture { - type Output = (T, Duration, usize); + type Output = (Result, Duration, usize); fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { let this = self.project(); @@ -80,7 +97,30 @@ impl> Future for CaptureFuture { EXTRA.with_borrow_mut(|cell| { *cell = Some(&mut data as *mut ThreadLocalData); }); - let result = this.future.poll(cx); + + let result = + panic::catch_unwind(panic::AssertUnwindSafe(|| this.future.poll(cx))).map_err(|err| { + let message = match err.downcast_ref::<&'static str>() { + Some(s) => TurboTasksExecutionErrorMessage::PIISafe(s), + None => match err.downcast_ref::() { + Some(s) => TurboTasksExecutionErrorMessage::NonPIISafe(s.clone()), + None => { + let error_message = err + .downcast_ref::>() + .map(|e| e.to_string()) + .unwrap_or_else(|| String::from("")); + + TurboTasksExecutionErrorMessage::NonPIISafe(error_message) + } + }, + }; + + LAST_ERROR_LOCATION.with_borrow(|loc| TurboTasksPanic { + message, + location: loc.clone(), + }) + }); + drop(guard); let elapsed = start.elapsed(); let allocations = start_allocations.until_now(); @@ -88,11 +128,15 @@ impl> Future for CaptureFuture { *this.allocations += allocations.allocations + data.allocations; *this.deallocations += allocations.deallocations + data.deallocations; match result { - Poll::Ready(r) => { + Err(err) => { + let memory_usage = this.allocations.saturating_sub(*this.deallocations); + Poll::Ready((Err(err), *this.duration, memory_usage)) + } + Ok(Poll::Ready(r)) => { let memory_usage = this.allocations.saturating_sub(*this.deallocations); - Poll::Ready((r, *this.duration, memory_usage)) + Poll::Ready((Ok(r), *this.duration, memory_usage)) } - Poll::Pending => Poll::Pending, + Ok(Poll::Pending) => Poll::Pending, } } } diff --git a/turbopack/crates/turbo-tasks/src/lib.rs b/turbopack/crates/turbo-tasks/src/lib.rs index 4c00012f52ea4..95b09b3c72947 100644 --- a/turbopack/crates/turbo-tasks/src/lib.rs +++ b/turbopack/crates/turbo-tasks/src/lib.rs @@ -28,6 +28,7 @@ #![feature(trivial_bounds)] #![feature(min_specialization)] +#![feature(thread_local)] #![feature(try_trait_v2)] #![deny(unsafe_op_in_unsafe_fn)] #![feature(result_flattening)] @@ -83,10 +84,11 @@ mod value; mod value_type; mod vc; -use std::hash::BuildHasherDefault; +use std::{cell::RefCell, hash::BuildHasherDefault, panic}; pub use anyhow::{Error, Result}; use auto_hash_map::AutoSet; +pub use capture_future::TurboTasksPanic; pub use collectibles::CollectiblesSource; pub use completion::{Completion, Completions}; pub use display::ValueToString; @@ -304,6 +306,19 @@ pub mod test_helpers { pub use super::manager::{current_task_for_testing, with_turbo_tasks_for_testing}; } +thread_local! { + /// The location of the last error that occurred in the current thread. + /// + /// Used for debugging when errors are sent to telemetry + pub(crate) static LAST_ERROR_LOCATION: RefCell> = const { RefCell::new(None) }; +} + +pub fn handle_panic(info: &panic::PanicHookInfo<'_>) { + LAST_ERROR_LOCATION.with_borrow_mut(|loc| { + *loc = info.location().map(|l| l.to_string()); + }); +} + pub fn register() { include!(concat!(env!("OUT_DIR"), "/register.rs")); } diff --git a/turbopack/crates/turbo-tasks/src/manager.rs b/turbopack/crates/turbo-tasks/src/manager.rs index 905a0c38d3b18..b859a95130dd1 100644 --- a/turbopack/crates/turbo-tasks/src/manager.rs +++ b/turbopack/crates/turbo-tasks/src/manager.rs @@ -4,7 +4,6 @@ use std::{ future::Future, hash::BuildHasherDefault, mem::take, - panic::AssertUnwindSafe, pin::Pin, sync::{ Arc, Mutex, RwLock, Weak, @@ -16,7 +15,6 @@ use std::{ use anyhow::{Result, anyhow}; use auto_hash_map::AutoMap; -use futures::FutureExt; use rustc_hash::FxHasher; use serde::{Deserialize, Serialize}; use tokio::{runtime::Handle, select, task_local}; @@ -30,7 +28,7 @@ use crate::{ VcValueType, backend::{ Backend, CachedTaskType, CellContent, TaskCollectiblesMap, TaskExecutionSpec, - TransientTaskType, TypedCellContent, + TransientTaskType, TurboTasksExecutionError, TypedCellContent, }, capture_future::{self, CaptureFuture}, event::{Event, EventListener}, @@ -44,7 +42,7 @@ use crate::{ task_statistics::TaskStatisticsApi, trace::TraceRawVcs, trait_helpers::get_trait_method, - util::{IdFactory, StaticOrArc}, + util::{IdFactory, SharedError, StaticOrArc}, vc::ReadVcFuture, }; @@ -694,8 +692,7 @@ impl TurboTasks { }; async { - let (result, duration, memory_usage) = - CaptureFuture::new(AssertUnwindSafe(future).catch_unwind()).await; + let (result, duration, memory_usage) = CaptureFuture::new(future).await; // wait for all spawned local tasks using `local` to finish let ltt = CURRENT_TASK_STATE @@ -703,13 +700,14 @@ impl TurboTasks { ltt.close(); ltt.wait().await; - let result = result.map_err(|any| match any.downcast::() { - Ok(owned) => Some(Cow::Owned(*owned)), - Err(any) => match any.downcast::<&'static str>() { - Ok(str) => Some(Cow::Borrowed(*str)), - Err(_) => None, - }, - }); + let result = match result { + Ok(Ok(raw_vc)) => Ok(raw_vc), + Ok(Err(err)) => Err(Arc::new(err.into())), + Err(err) => { + Err(Arc::new(TurboTasksExecutionError::Panic(Arc::new(err)))) + } + }; + this.backend.task_execution_result(task_id, result, &*this); let stateful = this.finish_current_task_state(); let cell_counters = CURRENT_TASK_STATE @@ -794,22 +792,30 @@ impl TurboTasks { let future = async move { let TaskExecutionSpec { future, span } = crate::task::local_task::get_local_task_execution_spec(&*this, &ty, persistence); + let ty = ty.clone(); async move { - let (result, _duration, _memory_usage) = - CaptureFuture::new(AssertUnwindSafe(future).catch_unwind()).await; - - let result = result.map_err(|any| match any.downcast::() { - Ok(owned) => Some(Cow::Owned(*owned)), - Err(any) => match any.downcast::<&'static str>() { - Ok(str) => Some(Cow::Borrowed(*str)), - Err(_) => None, - }, - }); + let (result, _duration, _memory_usage) = CaptureFuture::new(future).await; + + let result = match result { + Ok(Ok(raw_vc)) => Ok(raw_vc), + Ok(Err(err)) => Err(Arc::new(err.into())), + Err(err) => Err(Arc::new(TurboTasksExecutionError::Panic(Arc::new(err)))), + }; + let local_task = LocalTask::Done { output: match result { - Ok(Ok(raw_vc)) => OutputContent::Link(raw_vc), - Ok(Err(err)) => OutputContent::Error(err.into()), - Err(panic_err) => OutputContent::Panic(panic_err.map(Box::new)), + Ok(raw_vc) => OutputContent::Link(raw_vc), + Err(err) => match &*err { + TurboTasksExecutionError::Error { .. } => { + OutputContent::Error(SharedError::new( + anyhow::Error::new(err) + .context(format!("Execution of {ty} failed")), + )) + } + TurboTasksExecutionError::Panic(err) => { + OutputContent::Panic(err.clone()) + } + }, }, }; diff --git a/turbopack/crates/turbo-tasks/src/output.rs b/turbopack/crates/turbo-tasks/src/output.rs index b136d0d3ef2b2..cf32adfa56cec 100644 --- a/turbopack/crates/turbo-tasks/src/output.rs +++ b/turbopack/crates/turbo-tasks/src/output.rs @@ -1,27 +1,26 @@ use std::{ - borrow::Cow, fmt::{self, Display}, + sync::Arc, }; use anyhow::anyhow; -use crate::{RawVc, util::SharedError}; +use crate::{RawVc, TurboTasksPanic, util::SharedError}; /// A helper type representing the output of a resolved task. #[derive(Clone, Debug)] pub enum OutputContent { Link(RawVc), Error(SharedError), - Panic(Option>>), + Panic(Arc), } impl OutputContent { pub fn as_read_result(&self) -> anyhow::Result { match &self { - Self::Error(err) => Err(anyhow::Error::new(err.clone())), + Self::Error(err) => Err(anyhow!(err.clone())), Self::Link(raw_vc) => Ok(*raw_vc), - Self::Panic(Some(message)) => Err(anyhow!("A task panicked: {message}")), - Self::Panic(None) => Err(anyhow!("A task panicked")), + Self::Panic(err) => Err(anyhow!(err.clone())), } } } @@ -31,8 +30,7 @@ impl Display for OutputContent { match self { Self::Link(raw_vc) => write!(f, "link {raw_vc:?}"), Self::Error(err) => write!(f, "error {err}"), - Self::Panic(Some(message)) => write!(f, "panic {message}"), - Self::Panic(None) => write!(f, "panic"), + Self::Panic(err) => write!(f, "panic {err}"), } } } diff --git a/turbopack/crates/turbo-tasks/src/util.rs b/turbopack/crates/turbo-tasks/src/util.rs index f0b8f24fad4bc..92c5ec71faf60 100644 --- a/turbopack/crates/turbo-tasks/src/util.rs +++ b/turbopack/crates/turbo-tasks/src/util.rs @@ -23,11 +23,11 @@ pub use super::{ /// A error struct that is backed by an Arc to allow cloning errors #[derive(Debug, Clone)] pub struct SharedError { - inner: Arc, + inner: Arc, } impl SharedError { - pub fn new(err: Error) -> Self { + pub fn new(err: anyhow::Error) -> Self { Self { inner: Arc::new(err), } diff --git a/turbopack/crates/turbopack-tests/tests/snapshot/imports/json/issues/Code generation for chunk item errored-1c3f39.txt b/turbopack/crates/turbopack-tests/tests/snapshot/imports/json/issues/Code generation for chunk item errored-2c5cf4.txt similarity index 93% rename from turbopack/crates/turbopack-tests/tests/snapshot/imports/json/issues/Code generation for chunk item errored-1c3f39.txt rename to turbopack/crates/turbopack-tests/tests/snapshot/imports/json/issues/Code generation for chunk item errored-2c5cf4.txt index bbd784fc6b572..0a90933ef22ad 100644 --- a/turbopack/crates/turbopack-tests/tests/snapshot/imports/json/issues/Code generation for chunk item errored-1c3f39.txt +++ b/turbopack/crates/turbopack-tests/tests/snapshot/imports/json/issues/Code generation for chunk item errored-2c5cf4.txt @@ -6,6 +6,7 @@ error - [code gen] [project]/turbopack/crates/turbopack-tests/tests/snapshot/imp Debug info: - An error occurred while generating the chunk item [project]/turbopack/crates/turbopack-tests/tests/snapshot/imports/json/input/invalid.json (json) + - Execution of *EcmascriptChunkItemContent::module_factory failed - Execution of ::content failed - Unable to make a module from invalid JSON: expected `,` or `}` at line 3 column 26 at nested.? diff --git a/turbopack/crates/turbopack-tests/tests/snapshot/imports/json/output/turbopack_crates_turbopack-tests_tests_snapshot_imports_json_input_52abdb3f._.js b/turbopack/crates/turbopack-tests/tests/snapshot/imports/json/output/turbopack_crates_turbopack-tests_tests_snapshot_imports_json_input_52abdb3f._.js index 82cf0ccd41631..90a8810c36630 100644 --- a/turbopack/crates/turbopack-tests/tests/snapshot/imports/json/output/turbopack_crates_turbopack-tests_tests_snapshot_imports_json_input_52abdb3f._.js +++ b/turbopack/crates/turbopack-tests/tests/snapshot/imports/json/output/turbopack_crates_turbopack-tests_tests_snapshot_imports_json_input_52abdb3f._.js @@ -6,7 +6,7 @@ __turbopack_context__.v(JSON.parse("{\"name\":\"json-snapshot\"}"));}}), "[project]/turbopack/crates/turbopack-tests/tests/snapshot/imports/json/input/invalid.json (json)": (() => {{ -throw new Error("An error occurred while generating the chunk item [project]/turbopack/crates/turbopack-tests/tests/snapshot/imports/json/input/invalid.json (json)\n\nCaused by:\n- Unable to make a module from invalid JSON: expected `,` or `}` at line 3 column 26\n\nDebug info:\n- An error occurred while generating the chunk item [project]/turbopack/crates/turbopack-tests/tests/snapshot/imports/json/input/invalid.json (json)\n- Execution of ::content failed\n- Unable to make a module from invalid JSON: expected `,` or `}` at line 3 column 26\n at nested.?\n 1 | {\n 2 | \"nested\": {\n | v\n 3 + \"this-is\": \"invalid\" // lint-staged will remove trailing commas, so here's a comment\n | ^\n 4 | }\n 5 | }"); +throw new Error("An error occurred while generating the chunk item [project]/turbopack/crates/turbopack-tests/tests/snapshot/imports/json/input/invalid.json (json)\n\nCaused by:\n- Unable to make a module from invalid JSON: expected `,` or `}` at line 3 column 26\n\nDebug info:\n- An error occurred while generating the chunk item [project]/turbopack/crates/turbopack-tests/tests/snapshot/imports/json/input/invalid.json (json)\n- Execution of *EcmascriptChunkItemContent::module_factory failed\n- Execution of ::content failed\n- Unable to make a module from invalid JSON: expected `,` or `}` at line 3 column 26\n at nested.?\n 1 | {\n 2 | \"nested\": {\n | v\n 3 + \"this-is\": \"invalid\" // lint-staged will remove trailing commas, so here's a comment\n | ^\n 4 | }\n 5 | }"); }}), "[project]/turbopack/crates/turbopack-tests/tests/snapshot/imports/json/input/index.js [test] (ecmascript)": ((__turbopack_context__) => {