Skip to content

Commit 6c1d7e1

Browse files
wbinnssmithbgw
wbinnssmith
and
bgw
committed
turbo-tasks: Encode location information into panics
This is the basis of the Rust side of things to report location information with panics. To do: - [ ] Automated test Co-authored-by: Benjamin Woodruff <[email protected]>
1 parent 9914b0c commit 6c1d7e1

File tree

18 files changed

+275
-109
lines changed

18 files changed

+275
-109
lines changed

‎crates/napi/src/lib.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,19 @@ static ALLOC: dhat::Alloc = dhat::Alloc;
6868

6969
#[cfg(not(target_arch = "wasm32"))]
7070
#[napi::module_init]
71-
7271
fn init() {
72+
use std::panic::{set_hook, take_hook};
73+
7374
use tokio::runtime::Builder;
75+
use turbo_tasks::handle_panic;
7476
use turbo_tasks_malloc::TurboMalloc;
7577

78+
let prev_hook = take_hook();
79+
set_hook(Box::new(move |info| {
80+
handle_panic(info);
81+
prev_hook(info);
82+
}));
83+
7684
let rt = Builder::new_multi_thread()
7785
.enable_all()
7886
.on_thread_stop(|| {

‎turbopack/crates/turbo-tasks-backend/src/backend/mod.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ use tokio::time::{Duration, Instant};
2626
use turbo_tasks::{
2727
backend::{
2828
Backend, BackendJobId, CachedTaskType, CellContent, TaskExecutionSpec, TransientTaskRoot,
29-
TransientTaskType, TypedCellContent,
29+
TransientTaskType, TurboTasksExecutionError, TypedCellContent,
3030
},
3131
event::{Event, EventListener},
3232
registry::{self, get_value_type_global_name},
@@ -558,9 +558,7 @@ impl<B: BackingStorage> TurboTasksBackendInner<B> {
558558
let result = match output {
559559
OutputValue::Cell(cell) => Some(Ok(Ok(RawVc::TaskCell(cell.task, cell.cell)))),
560560
OutputValue::Output(task) => Some(Ok(Ok(RawVc::TaskOutput(*task)))),
561-
OutputValue::Error | OutputValue::Panic => {
562-
get!(task, Error).map(|error| Err(error.clone().into()))
563-
}
561+
OutputValue::Error => get!(task, Error).map(|error| Err(error.clone().into())),
564562
};
565563
if let Some(result) = result {
566564
if self.should_track_dependencies() {
@@ -1411,7 +1409,7 @@ impl<B: BackingStorage> TurboTasksBackendInner<B> {
14111409
fn task_execution_result(
14121410
&self,
14131411
task_id: TaskId,
1414-
result: Result<Result<RawVc>, Option<Cow<'static, str>>>,
1412+
result: Result<RawVc, Arc<TurboTasksExecutionError>>,
14151413
turbo_tasks: &dyn TurboTasksBackendApi<TurboTasksBackend<B>>,
14161414
) {
14171415
operation::UpdateOutputOperation::run(task_id, result, self.execute_context(turbo_tasks));
@@ -2343,7 +2341,7 @@ impl<B: BackingStorage> Backend for TurboTasksBackend<B> {
23432341
fn task_execution_result(
23442342
&self,
23452343
task_id: TaskId,
2346-
result: Result<Result<RawVc>, Option<Cow<'static, str>>>,
2344+
result: Result<RawVc, Arc<TurboTasksExecutionError>>,
23472345
turbo_tasks: &dyn TurboTasksBackendApi<Self>,
23482346
) {
23492347
self.0.task_execution_result(task_id, result, turbo_tasks);

‎turbopack/crates/turbo-tasks-backend/src/backend/operation/update_output.rs

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
use std::{borrow::Cow, mem::take};
1+
use std::{mem::take, sync::Arc};
22

3-
use anyhow::{anyhow, Result};
3+
use anyhow::Result;
44
use serde::{Deserialize, Serialize};
5-
use turbo_tasks::{util::SharedError, RawVc, TaskId};
5+
use turbo_tasks::{backend::TurboTasksExecutionError, util::SharedError, RawVc, TaskId};
66

77
#[cfg(feature = "trace_task_dirty")]
88
use crate::backend::operation::invalidate::TaskDirtyCause;
@@ -44,7 +44,7 @@ pub enum UpdateOutputOperation {
4444
impl UpdateOutputOperation {
4545
pub fn run(
4646
task_id: TaskId,
47-
output: Result<Result<RawVc>, Option<Cow<'static, str>>>,
47+
output: Result<RawVc, Arc<TurboTasksExecutionError>>,
4848
mut ctx: impl ExecuteContext,
4949
) {
5050
let mut task = ctx.task(task_id, TaskDataCategory::All);
@@ -68,15 +68,15 @@ impl UpdateOutputOperation {
6868
let old_error = task.remove(&CachedDataItemKey::Error {});
6969
let current_output = get!(task, Output);
7070
let output_value = match output {
71-
Ok(Ok(RawVc::TaskOutput(output_task_id))) => {
71+
Ok(RawVc::TaskOutput(output_task_id)) => {
7272
if let Some(OutputValue::Output(current_task_id)) = current_output {
7373
if *current_task_id == output_task_id {
7474
return;
7575
}
7676
}
7777
OutputValue::Output(output_task_id)
7878
}
79-
Ok(Ok(RawVc::TaskCell(output_task_id, cell))) => {
79+
Ok(RawVc::TaskCell(output_task_id, cell)) => {
8080
if let Some(OutputValue::Cell(CellRef {
8181
task: current_task_id,
8282
cell: current_cell,
@@ -91,28 +91,18 @@ impl UpdateOutputOperation {
9191
cell,
9292
})
9393
}
94-
Ok(Ok(RawVc::LocalOutput(..))) => {
94+
Ok(RawVc::LocalOutput(..)) => {
9595
panic!("Non-local tasks must not return a local Vc");
9696
}
97-
Ok(Err(err)) => {
97+
Err(err) => {
9898
task.insert(CachedDataItem::Error {
99-
value: SharedError::new(err.context(format!(
99+
value: SharedError::new(anyhow::Error::new(err).context(format!(
100100
"Execution of {} failed",
101101
ctx.get_task_description(task_id)
102102
))),
103103
});
104104
OutputValue::Error
105105
}
106-
Err(panic) => {
107-
task.insert(CachedDataItem::Error {
108-
value: SharedError::new(anyhow!(
109-
"Panic in {}: {:?}",
110-
ctx.get_task_description(task_id),
111-
panic
112-
)),
113-
});
114-
OutputValue::Panic
115-
}
116106
};
117107
let old_content = task.insert(CachedDataItem::Output {
118108
value: output_value,

‎turbopack/crates/turbo-tasks-backend/src/data.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,15 +57,13 @@ pub enum OutputValue {
5757
Cell(CellRef),
5858
Output(TaskId),
5959
Error,
60-
Panic,
6160
}
6261
impl OutputValue {
6362
fn is_transient(&self) -> bool {
6463
match self {
6564
OutputValue::Cell(cell) => cell.task.is_transient(),
6665
OutputValue::Output(task) => task.is_transient(),
6766
OutputValue::Error => false,
68-
OutputValue::Panic => false,
6967
}
7068
}
7169
}

‎turbopack/crates/turbo-tasks-backend/tests/panics.rs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
use core::panic;
2+
use std::{
3+
panic::{set_hook, take_hook},
4+
sync::{Arc, LazyLock},
5+
};
6+
7+
use anyhow::Result;
8+
use regex::Regex;
9+
use turbo_tasks::{backend::TurboTasksExecutionError, handle_panic, Vc};
10+
use turbo_tasks_testing::{register, run_without_cache_check, Registration};
11+
12+
static REGISTRATION: Registration = register!();
13+
14+
static FILE_PATH_REGEX: LazyLock<Regex> =
15+
LazyLock::new(|| Regex::new(r"panics\.rs:\d+:\d+$").unwrap());
16+
17+
#[tokio::test]
18+
async fn test_panics_include_location() {
19+
let prev_hook = take_hook();
20+
set_hook(Box::new(move |info| {
21+
handle_panic(info);
22+
prev_hook(info);
23+
}));
24+
25+
let result =
26+
run_without_cache_check(&REGISTRATION, async move { anyhow::Ok(*double(3).await?) }).await;
27+
28+
let error = result.unwrap_err();
29+
let root_cause = error.root_cause();
30+
31+
let Some(panic) = root_cause.downcast_ref::<Arc<TurboTasksExecutionError>>() else {
32+
panic!("Expected a TurboTasksExecutionError");
33+
};
34+
35+
let TurboTasksExecutionError::Panic(panic) = &**panic else {
36+
panic!("Expected a TurboTasksExecutionError::Panic");
37+
};
38+
39+
assert!(
40+
FILE_PATH_REGEX.is_match(panic.location.as_ref().unwrap()),
41+
"Panic location '{}' should be a line in panics.rs",
42+
panic.location.as_ref().unwrap()
43+
);
44+
}
45+
46+
#[turbo_tasks::function]
47+
fn double(_val: u64) -> Result<Vc<u64>> {
48+
panic!("oh no");
49+
}

‎turbopack/crates/turbo-tasks-backend/tests/trace_transient.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,9 @@ async fn test_trace_transient() {
3232
anyhow::Ok(())
3333
})
3434
.await;
35-
assert!(result
36-
.unwrap_err()
37-
.to_string()
38-
.contains(&EXPECTED_TRACE.escape_debug().to_string()));
35+
36+
let message = format!("{:#}", result.unwrap_err());
37+
assert!(message.contains(&EXPECTED_TRACE.to_string()));
3938
}
4039

4140
#[turbo_tasks::value]

‎turbopack/crates/turbo-tasks-backend/tests/transient_collectible.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,9 @@ async fn test_transient_emit_from_persistent() {
1919
anyhow::Ok(())
2020
})
2121
.await;
22-
assert!(result
23-
.unwrap_err()
24-
.to_string()
25-
.contains(&EXPECTED_MSG.escape_debug().to_string()));
22+
23+
let message = format!("{:#}", result.unwrap_err());
24+
assert!(message.contains(&EXPECTED_MSG.to_string()));
2625
}
2726

2827
#[turbo_tasks::function(operation)]

‎turbopack/crates/turbo-tasks-memory/src/memory_backend.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use std::{
2-
borrow::{Borrow, Cow},
2+
borrow::Borrow,
33
future::Future,
44
hash::{BuildHasher, BuildHasherDefault, Hash},
55
num::NonZeroU32,
@@ -20,7 +20,7 @@ use turbo_prehash::{BuildHasherExt, PassThroughHash, PreHashed};
2020
use turbo_tasks::{
2121
backend::{
2222
Backend, BackendJobId, CachedTaskType, CellContent, TaskCollectiblesMap, TaskExecutionSpec,
23-
TransientTaskType, TypedCellContent,
23+
TransientTaskType, TurboTasksExecutionError, TypedCellContent,
2424
},
2525
event::EventListener,
2626
task_statistics::TaskStatisticsApi,
@@ -415,12 +415,12 @@ impl Backend for MemoryBackend {
415415
fn task_execution_result(
416416
&self,
417417
task_id: TaskId,
418-
result: Result<Result<RawVc>, Option<Cow<'static, str>>>,
418+
result: Result<RawVc, Arc<TurboTasksExecutionError>>,
419419
turbo_tasks: &dyn TurboTasksBackendApi<MemoryBackend>,
420420
) {
421421
self.with_task(task_id, |task| {
422422
#[cfg(debug_assertions)]
423-
if let Ok(Ok(RawVc::TaskOutput(result))) = result.as_ref() {
423+
if let Ok(RawVc::TaskOutput(result)) = result.as_ref() {
424424
if *result == task_id {
425425
panic!("Task {} returned itself as output", task.get_description());
426426
}

‎turbopack/crates/turbo-tasks-memory/src/output.rs

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::{borrow::Cow, fmt::Debug, mem::take};
1+
use std::{fmt::Debug, mem::take};
22

33
use anyhow::{anyhow, Error, Result};
44
use turbo_tasks::{
@@ -51,18 +51,6 @@ impl Output {
5151
}
5252
}
5353

54-
pub fn panic(
55-
&mut self,
56-
message: Option<Cow<'static, str>>,
57-
turbo_tasks: &dyn TurboTasksBackendApi<MemoryBackend>,
58-
) {
59-
self.content = Some(OutputContent::Panic(message.map(Box::new)));
60-
// notify
61-
if !self.dependent_tasks.is_empty() {
62-
turbo_tasks.schedule_notify_tasks_set(&take(&mut self.dependent_tasks));
63-
}
64-
}
65-
6654
pub fn gc_drop(self, turbo_tasks: &dyn TurboTasksBackendApi<MemoryBackend>) {
6755
// notify
6856
if !self.dependent_tasks.is_empty() {

‎turbopack/crates/turbo-tasks-memory/src/task.rs

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
use std::{
2-
borrow::Cow,
32
fmt::{self, Debug, Display, Formatter},
43
future::Future,
54
hash::{BuildHasherDefault, Hash},
@@ -19,7 +18,10 @@ use smallvec::SmallVec;
1918
use tracing::Span;
2019
use turbo_prehash::PreHashed;
2120
use turbo_tasks::{
22-
backend::{CachedTaskType, CellContent, TaskCollectiblesMap, TaskExecutionSpec},
21+
backend::{
22+
CachedTaskType, CellContent, TaskCollectiblesMap, TaskExecutionSpec,
23+
TurboTasksExecutionError,
24+
},
2325
event::{Event, EventListener},
2426
get_invalidator, registry, CellId, Invalidator, RawVc, ReadConsistency, TaskId, TaskIdSet,
2527
TraitTypeId, TurboTasksBackendApi, TurboTasksBackendApiExt, ValueTypeId,
@@ -839,7 +841,7 @@ impl Task {
839841

840842
pub(crate) fn execution_result(
841843
&self,
842-
result: Result<Result<RawVc>, Option<Cow<'static, str>>>,
844+
result: Result<RawVc, Arc<TurboTasksExecutionError>>,
843845
backend: &MemoryBackend,
844846
turbo_tasks: &dyn TurboTasksBackendApi<MemoryBackend>,
845847
) {
@@ -851,7 +853,7 @@ impl Task {
851853
// TODO maybe this should be controlled by a heuristic
852854
}
853855
InProgress(..) => match result {
854-
Ok(Ok(result)) => {
856+
Ok(result) => {
855857
if state.output != result {
856858
if backend.print_task_invalidation && state.output.content.is_some() {
857859
println!(
@@ -867,13 +869,17 @@ impl Task {
867869
state.output.link(result, turbo_tasks)
868870
}
869871
}
870-
Ok(Err(mut err)) => {
871-
if let Some(name) = self.get_function_name() {
872-
err = err.context(format!("Execution of {} failed", name));
873-
}
872+
Err(err) => {
873+
let err = anyhow::Error::new(err).context(
874+
if let Some(name) = self.get_function_name() {
875+
format!("Execution of {} failed", name)
876+
} else {
877+
"Execution failed".to_string()
878+
},
879+
);
880+
874881
state.output.error(err, turbo_tasks)
875882
}
876-
Err(message) => state.output.panic(message, turbo_tasks),
877883
},
878884

879885
Dirty { .. } | Scheduled { .. } | Done { .. } => {

0 commit comments

Comments
 (0)