Skip to content

Commit 77c4713

Browse files
feat(turborepo): Convert even more errors (#7513)
### Description Converting more errors to be pretty. Involves some plumbing and bookkeeping. ### Testing Instructions Current tests around task validation should suffice. Closes TURBO-2433 --------- Co-authored-by: Mehul Kar <[email protected]>
1 parent f790c14 commit 77c4713

File tree

17 files changed

+222
-62
lines changed

17 files changed

+222
-62
lines changed

crates/turborepo-lib/src/cli/error.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ pub enum Error {
3737
#[error(transparent)]
3838
Generate(#[from] generate::Error),
3939
#[error(transparent)]
40+
#[diagnostic(transparent)]
4041
Prune(#[from] prune::Error),
4142
#[error(transparent)]
4243
PackageJson(#[from] turborepo_repository::package_json::Error),

crates/turborepo-lib/src/commands/prune.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use std::os::unix::fs::PermissionsExt;
33
use std::sync::OnceLock;
44

55
use lazy_static::lazy_static;
6+
use miette::Diagnostic;
67
use tracing::trace;
78
use turbopath::{
89
AbsoluteSystemPathBuf, AnchoredSystemPath, AnchoredSystemPathBuf, RelativeUnixPath,
@@ -19,7 +20,7 @@ use crate::turbo_json::RawTurboJson;
1920

2021
pub const DEFAULT_OUTPUT_DIR: &str = "out";
2122

22-
#[derive(Debug, thiserror::Error)]
23+
#[derive(Debug, thiserror::Error, Diagnostic)]
2324
pub enum Error {
2425
#[error("io error while pruning: {0}")]
2526
Io(#[from] std::io::Error),
@@ -30,6 +31,7 @@ pub enum Error {
3031
#[error("path error while pruning: {0}")]
3132
Path(#[from] turbopath::PathError),
3233
#[error(transparent)]
34+
#[diagnostic(transparent)]
3335
TurboJsonParser(#[from] crate::turbo_json::parser::Error),
3436
#[error(transparent)]
3537
PackageJson(#[from] turborepo_repository::package_json::Error),

crates/turborepo-lib/src/engine/builder.rs

Lines changed: 53 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,24 @@ pub enum Error {
5050
#[source_code]
5151
text: NamedSource,
5252
},
53-
#[error("Could not find workspace \"{package}\" from task \"{task_id}\" in project")]
54-
MissingWorkspaceFromTask { package: String, task_id: String },
55-
#[error("Could not find \"{task_id}\" in root turbo.json or \"{task_name}\" in workspace")]
56-
MissingWorkspaceTask { task_id: String, task_name: String },
53+
#[error("Could not find package \"{package}\" from task \"{task_id}\" in project")]
54+
MissingPackageFromTask {
55+
#[label]
56+
span: Option<SourceSpan>,
57+
#[source_code]
58+
text: NamedSource,
59+
package: String,
60+
task_id: String,
61+
},
62+
#[error("Could not find \"{task_id}\" in root turbo.json or \"{task_name}\" in package")]
63+
MissingPackageTask {
64+
#[label]
65+
span: Option<SourceSpan>,
66+
#[source_code]
67+
text: NamedSource,
68+
task_id: String,
69+
task_name: String,
70+
},
5771
#[error(transparent)]
5872
#[diagnostic(transparent)]
5973
Config(#[from] crate::config::Error),
@@ -64,8 +78,15 @@ pub enum Error {
6478
},
6579
#[error(transparent)]
6680
Graph(#[from] graph::Error),
67-
#[error("Invalid task name {task_name}: {reason}")]
68-
InvalidTaskName { task_name: String, reason: String },
81+
#[error("invalid task name: {reason}")]
82+
InvalidTaskName {
83+
#[label]
84+
span: Option<SourceSpan>,
85+
#[source_code]
86+
text: NamedSource,
87+
task_name: String,
88+
reason: String,
89+
},
6990
}
7091

7192
pub struct EngineBuilder<'a> {
@@ -160,7 +181,8 @@ impl<'a> EngineBuilder<'a> {
160181
// workspace is acceptable)
161182
if !matches!(workspace, PackageName::Root) || self.root_enabled_tasks.contains(task)
162183
{
163-
traversal_queue.push_back(task.to(task_id));
184+
let task_id = task.to(task_id);
185+
traversal_queue.push_back(task_id);
164186
}
165187
}
166188
}
@@ -187,6 +209,11 @@ impl<'a> EngineBuilder<'a> {
187209
let mut engine = Engine::default();
188210

189211
while let Some(task_id) = traversal_queue.pop_front() {
212+
{
213+
let (task_id, span) = task_id.clone().split();
214+
engine.add_task_location(task_id.into_owned(), span);
215+
}
216+
190217
if task_id.package() == ROOT_PKG_NAME
191218
&& !self
192219
.root_enabled_tasks
@@ -200,7 +227,7 @@ impl<'a> EngineBuilder<'a> {
200227
});
201228
}
202229

203-
validate_task_name(task_id.task())?;
230+
validate_task_name(task_id.to(task_id.task()))?;
204231

205232
if task_id.package() != ROOT_PKG_NAME
206233
&& self
@@ -210,9 +237,12 @@ impl<'a> EngineBuilder<'a> {
210237
{
211238
// If we have a pkg it should be in PackageGraph.
212239
// If we're hitting this error something has gone wrong earlier when building
213-
// PackageGraph or the workspace really doesn't exist and
240+
// PackageGraph or the package really doesn't exist and
214241
// turbo.json is misconfigured.
215-
return Err(Error::MissingWorkspaceFromTask {
242+
let (span, text) = task_id.span_and_text("turbo.json");
243+
return Err(Error::MissingPackageFromTask {
244+
span,
245+
text,
216246
package: task_id.package().to_string(),
217247
task_id: task_id.to_string(),
218248
});
@@ -275,7 +305,8 @@ impl<'a> EngineBuilder<'a> {
275305
engine
276306
.task_graph
277307
.add_edge(to_task_index, from_task_index, ());
278-
traversal_queue.push_back(span.to(from_task_id));
308+
let from_task_id = span.to(from_task_id);
309+
traversal_queue.push_back(from_task_id);
279310
}
280311
});
281312

@@ -289,11 +320,11 @@ impl<'a> EngineBuilder<'a> {
289320
engine
290321
.task_graph
291322
.add_edge(to_task_index, from_task_index, ());
292-
traversal_queue.push_back(span.to(from_task_id));
323+
let from_task_id = span.to(from_task_id);
324+
traversal_queue.push_back(from_task_id);
293325
}
294326

295327
engine.add_definition(task_id.as_inner().clone().into_owned(), task_definition);
296-
297328
if !has_deps && !has_topo_deps {
298329
engine.connect_to_root(&to_task_id);
299330
}
@@ -395,7 +426,10 @@ impl<'a> EngineBuilder<'a> {
395426
}
396427

397428
if task_definitions.is_empty() {
398-
return Err(Error::MissingWorkspaceTask {
429+
let (span, text) = task_id.span_and_text("turbo.json");
430+
return Err(Error::MissingPackageTask {
431+
span,
432+
text,
399433
task_id: task_id.to_string(),
400434
task_name: task_name.to_string(),
401435
});
@@ -447,12 +481,15 @@ impl Error {
447481
// we can expand the patterns here.
448482
const INVALID_TOKENS: &[&str] = &["$colon$"];
449483

450-
fn validate_task_name(task: &str) -> Result<(), Error> {
484+
fn validate_task_name(task: Spanned<&str>) -> Result<(), Error> {
451485
INVALID_TOKENS
452486
.iter()
453487
.find(|token| task.contains(**token))
454488
.map(|found_token| {
489+
let (span, text) = task.span_and_text("turbo.json");
455490
Err(Error::InvalidTaskName {
491+
span,
492+
text,
456493
task_name: task.to_string(),
457494
reason: format!("task contains invalid string '{found_token}'"),
458495
})
@@ -1106,7 +1143,7 @@ mod test {
11061143
#[test_case("build:prod", None)]
11071144
#[test_case("build$colon$prod", Some("task contains invalid string '$colon$'"))]
11081145
fn test_validate_task_name(task_name: &str, reason: Option<&str>) {
1109-
let result = validate_task_name(task_name)
1146+
let result = validate_task_name(Spanned::new(task_name))
11101147
.map_err(|e| {
11111148
if let Error::InvalidTaskName { reason, .. } = e {
11121149
reason

crates/turborepo-lib/src/engine/mod.rs

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,10 @@ use std::{
1111

1212
pub use builder::{EngineBuilder, Error as BuilderError};
1313
pub use execute::{ExecuteError, ExecutionOptions, Message, StopExecution};
14-
use miette::Diagnostic;
14+
use miette::{Diagnostic, NamedSource, SourceSpan};
1515
use petgraph::Graph;
1616
use thiserror::Error;
17+
use turborepo_errors::Spanned;
1718
use turborepo_repository::package_graph::{PackageGraph, PackageName};
1819

1920
use crate::{run::task_id::TaskId, task_graph::TaskDefinition};
@@ -42,6 +43,7 @@ pub struct Engine<S = Built> {
4243
root_index: petgraph::graph::NodeIndex,
4344
task_lookup: HashMap<TaskId<'static>, petgraph::graph::NodeIndex>,
4445
task_definitions: HashMap<TaskId<'static>, TaskDefinition>,
46+
task_locations: HashMap<TaskId<'static>, Spanned<()>>,
4547
}
4648

4749
impl Engine<Building> {
@@ -54,6 +56,7 @@ impl Engine<Building> {
5456
root_index,
5557
task_lookup: HashMap::default(),
5658
task_definitions: HashMap::default(),
59+
task_locations: HashMap::default(),
5760
}
5861
}
5962

@@ -78,13 +81,27 @@ impl Engine<Building> {
7881
self.task_definitions.insert(task_id, definition)
7982
}
8083

84+
pub fn add_task_location(&mut self, task_id: TaskId<'static>, location: Spanned<()>) {
85+
// If we don't have the location stored,
86+
// or if the location stored is empty, we add it to the map.
87+
let has_location = self
88+
.task_locations
89+
.get(&task_id)
90+
.map_or(false, |existing| existing.range.is_some());
91+
92+
if !has_location {
93+
self.task_locations.insert(task_id, location);
94+
}
95+
}
96+
8197
// Seals the task graph from being mutated
8298
pub fn seal(self) -> Engine<Built> {
8399
let Engine {
84100
task_graph,
85101
task_lookup,
86102
root_index,
87103
task_definitions,
104+
task_locations,
88105
..
89106
} = self;
90107
Engine {
@@ -93,6 +110,7 @@ impl Engine<Building> {
93110
task_lookup,
94111
root_index,
95112
task_definitions,
113+
task_locations,
96114
}
97115
}
98116
}
@@ -192,7 +210,15 @@ impl Engine<Built> {
192210
if task_definition.persistent
193211
&& package_json.scripts.contains_key(dep_id.task())
194212
{
213+
let (span, text) = self
214+
.task_locations
215+
.get(dep_id)
216+
.map(|spanned| spanned.span_and_text("turbo.json"))
217+
.unwrap_or((None, NamedSource::new("", "")));
218+
195219
return Err(ValidateError::DependencyOnPersistentTask {
220+
span,
221+
text,
196222
persistent_task: dep_id.to_string(),
197223
dependant: task_id.to_string(),
198224
});
@@ -254,6 +280,10 @@ pub enum ValidateError {
254280
MissingPackageJson { package: String },
255281
#[error("\"{persistent_task}\" is a persistent task, \"{dependant}\" cannot depend on it")]
256282
DependencyOnPersistentTask {
283+
#[label("persistent task")]
284+
span: Option<SourceSpan>,
285+
#[source_code]
286+
text: NamedSource,
257287
persistent_task: String,
258288
dependant: String,
259289
},

crates/turborepo-lib/src/run/error.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,17 @@ use turborepo_repository::package_graph;
44

55
use super::graph_visualizer;
66
use crate::{
7-
config, daemon, engine, opts,
7+
config, daemon, engine,
8+
engine::ValidateError,
9+
opts,
810
run::{global_hash, scope},
911
task_graph, task_hash,
1012
};
1113

1214
#[derive(Debug, Error, Diagnostic)]
1315
pub enum Error {
14-
#[error("error preparing engine: Invalid persistent task configuration:\n{0}")]
15-
EngineValidation(String),
16+
#[error("invalid persistent task configuration")]
17+
EngineValidation(#[related] Vec<ValidateError>),
1618
#[error(transparent)]
1719
Graph(#[from] graph_visualizer::Error),
1820
#[error(transparent)]

crates/turborepo-lib/src/run/mod.rs

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ use std::{
1919

2020
pub use cache::{ConfigCache, RunCache, TaskCache};
2121
use chrono::{DateTime, Local};
22-
use itertools::Itertools;
2322
use rayon::iter::ParallelBridge;
2423
use tracing::debug;
2524
use turbopath::{AbsoluteSystemPathBuf, AnchoredSystemPath};
@@ -586,15 +585,7 @@ impl Run {
586585
if !self.opts.run_opts.parallel {
587586
engine
588587
.validate(pkg_dep_graph, self.opts.run_opts.concurrency)
589-
.map_err(|errors| {
590-
Error::EngineValidation(
591-
errors
592-
.into_iter()
593-
.map(|e| e.to_string())
594-
.sorted()
595-
.join("\n"),
596-
)
597-
})?;
588+
.map_err(Error::EngineValidation)?;
598589
}
599590

600591
Ok(engine)

turborepo-tests/integration/tests/persistent-dependencies/1-topological.t

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,15 @@
1313
// app-a#dev
1414
// └── pkg-a#dev
1515
$ ${TURBO} run dev
16-
x error preparing engine: Invalid persistent task configuration:
17-
| "pkg-a#dev" is a persistent task, "app-a#dev" cannot depend on it
16+
x invalid persistent task configuration
17+
18+
Error: x "pkg-a#dev" is a persistent task, "app-a#dev" cannot depend on it
19+
,-[turbo.json:4:1]
20+
4 | "dev": {
21+
5 | "dependsOn": ["^dev"],
22+
: ^^^|^^
23+
: `-- persistent task
24+
6 | "persistent": true
25+
`----
1826

1927
[1]

turborepo-tests/integration/tests/persistent-dependencies/10-too-many.t

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,17 @@
22
$ . ${TESTDIR}/../../../helpers/setup_integration_test.sh persistent_dependencies/10-too-many
33

44
$ ${TURBO} run build --concurrency=1
5-
x error preparing engine: Invalid persistent task configuration:
6-
| You have 2 persistent tasks but `turbo` is configured for concurrency of
5+
x invalid persistent task configuration
6+
7+
Error: x You have 2 persistent tasks but `turbo` is configured for concurrency of
78
| 1. Set --concurrency to at least 3
89

910
[1]
1011

1112
$ ${TURBO} run build --concurrency=2
12-
x error preparing engine: Invalid persistent task configuration:
13-
| You have 2 persistent tasks but `turbo` is configured for concurrency of
13+
x invalid persistent task configuration
14+
15+
Error: x You have 2 persistent tasks but `turbo` is configured for concurrency of
1416
| 2. Set --concurrency to at least 3
1517

1618
[1]

turborepo-tests/integration/tests/persistent-dependencies/2-same-workspace.t

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,15 @@
1313
// └── app-a#dev
1414
//
1515
$ ${TURBO} run build
16-
x error preparing engine: Invalid persistent task configuration:
17-
| "app-a#dev" is a persistent task, "app-a#build" cannot depend on it
16+
x invalid persistent task configuration
17+
18+
Error: x "app-a#dev" is a persistent task, "app-a#build" cannot depend on it
19+
,-[turbo.json:4:1]
20+
4 | "build": {
21+
5 | "dependsOn": ["dev"]
22+
: ^^|^^
23+
: `-- persistent task
24+
6 | },
25+
`----
1826

1927
[1]

0 commit comments

Comments
 (0)