Skip to content

Commit

Permalink
feat(trace): pretty print trace errors to logs (#9367)
Browse files Browse the repository at this point in the history
### Description

To help debug trace, pretty print the errors to stderr by default.
Includes fancy error messages either via SWC or our own miette
infrastructure

### Testing Instructions

Try using trace and hit an error
  • Loading branch information
NicholasLYang authored Nov 5, 2024
1 parent 3d9b259 commit baa4eb5
Show file tree
Hide file tree
Showing 8 changed files with 78 additions and 25 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions crates/turbo-trace/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@ futures = { workspace = true }
globwalk = { version = "0.1.0", path = "../turborepo-globwalk" }
miette = { workspace = true, features = ["fancy"] }
oxc_resolver = { version = "2.0.0" }
swc_common = { workspace = true, features = ["concurrent"] }
swc_common = { workspace = true, features = ["concurrent", "tty-emitter"] }
swc_ecma_ast = { workspace = true }
swc_ecma_parser = { workspace = true }
swc_ecma_visit = { workspace = true }
thiserror = { workspace = true }
tokio = { workspace = true }
tokio = { workspace = true, features = ["full"] }
tracing = { workspace = true }
tracing-subscriber = { workspace = true }
turbopath = { workspace = true }
Expand Down
72 changes: 58 additions & 14 deletions crates/turbo-trace/src/tracer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,33 @@ use std::{collections::HashMap, sync::Arc};

use camino::Utf8PathBuf;
use globwalk::WalkType;
use miette::{Diagnostic, NamedSource, SourceSpan};
use miette::{Diagnostic, Report, SourceSpan};
use oxc_resolver::{
EnforceExtension, ResolveError, ResolveOptions, Resolver, TsconfigOptions, TsconfigReferences,
};
use swc_common::{comments::SingleThreadedComments, input::StringInput, FileName, SourceMap};
use swc_common::{
comments::SingleThreadedComments,
errors::{ColorConfig, Handler},
input::StringInput,
FileName, SourceMap,
};
use swc_ecma_ast::EsVersion;
use swc_ecma_parser::{lexer::Lexer, Capturing, EsSyntax, Parser, Syntax, TsSyntax};
use swc_ecma_visit::VisitWith;
use thiserror::Error;
use tokio::task::JoinSet;
use tracing::debug;
use tracing::{debug, error};
use turbopath::{AbsoluteSystemPath, AbsoluteSystemPathBuf, PathError};

use crate::import_finder::ImportFinder;

#[derive(Debug, Default)]
pub struct SeenFile {
// We have to add these because of a Rust bug where dead code analysis
// doesn't work properly in multi-target crates
// (i.e. crates with both a binary and library)
// https://github.com/rust-lang/rust/issues/95513
#[allow(dead_code)]
pub ast: Option<swc_ecma_ast::Module>,
}

Expand All @@ -31,35 +41,61 @@ pub struct Tracer {
import_type: ImportType,
}

#[derive(Debug, Error, Diagnostic)]
#[derive(Clone, Debug, Error, Diagnostic)]
pub enum TraceError {
#[error("failed to parse file {}: {:?}", .0, .1)]
ParseError(AbsoluteSystemPathBuf, swc_ecma_parser::error::Error),
#[error("failed to read file: {0}")]
FileNotFound(AbsoluteSystemPathBuf),
#[error(transparent)]
PathEncoding(PathError),
PathEncoding(Arc<PathError>),
#[error("tracing a root file `{0}`, no parent found")]
RootFile(AbsoluteSystemPathBuf),
#[error("failed to resolve import to `{path}`")]
#[error("failed to resolve import to `{import}` in `{file_path}`")]
Resolve {
path: String,
import: String,
file_path: String,
#[label("import here")]
span: SourceSpan,
#[source_code]
text: NamedSource,
text: String,
},
#[error("failed to walk files")]
GlobError(#[from] globwalk::WalkError),
GlobError(Arc<globwalk::WalkError>),
}

impl TraceResult {
#[allow(dead_code)]
pub fn emit_errors(&self) {
let handler = Handler::with_tty_emitter(
ColorConfig::Auto,
true,
false,
Some(self.source_map.clone()),
);
for error in &self.errors {
match error {
TraceError::ParseError(_, e) => {
e.clone().into_diagnostic(&handler).emit();
}
e => {
eprintln!("{:?}", Report::new(e.clone()));
}
}
}
}
}

pub struct TraceResult {
#[allow(dead_code)]
source_map: Arc<SourceMap>,
pub errors: Vec<TraceError>,
pub files: HashMap<AbsoluteSystemPathBuf, SeenFile>,
}

/// The type of imports to trace.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
#[allow(dead_code)]
pub enum ImportType {
/// Trace all imports.
All,
Expand Down Expand Up @@ -90,6 +126,7 @@ impl Tracer {
}
}

#[allow(dead_code)]
pub fn set_import_type(&mut self, import_type: ImportType) {
self.import_type = import_type;
}
Expand Down Expand Up @@ -162,7 +199,7 @@ impl Tracer {
match resolver.resolve(file_dir, import) {
Ok(resolved) => {
debug!("resolved {:?}", resolved);
match resolved.into_path_buf().try_into() {
match resolved.into_path_buf().try_into().map_err(Arc::new) {
Ok(path) => files.push(path),
Err(err) => {
errors.push(TraceError::PathEncoding(err));
Expand All @@ -176,11 +213,14 @@ impl Tracer {
Err(err) => {
debug!("failed to resolve: {:?}", err);
let (start, end) = source_map.span_to_char_offset(&source_file, *span);
let start = start as usize;
let end = end as usize;

errors.push(TraceError::Resolve {
path: import.to_string(),
span: (start as usize, end as usize).into(),
text: NamedSource::new(file_path.to_string(), file_content.clone()),
import: import.to_string(),
file_path: file_path.to_string(),
span: SourceSpan::new(start.into(), (end - start).into()),
text: file_content.clone(),
});
continue;
}
Expand Down Expand Up @@ -299,6 +339,7 @@ impl Tracer {
}

TraceResult {
source_map: self.source_map.clone(),
files: seen,
errors: self.errors,
}
Expand All @@ -322,15 +363,17 @@ impl Tracer {
Ok(files) => files,
Err(e) => {
return TraceResult {
source_map: self.source_map.clone(),
files: HashMap::new(),
errors: vec![e.into()],
errors: vec![TraceError::GlobError(Arc::new(e))],
}
}
};

let mut futures = JoinSet::new();

let resolver = Arc::new(self.create_resolver());
let source_map = self.source_map.clone();
let shared_self = Arc::new(self);

for file in files {
Expand Down Expand Up @@ -380,6 +423,7 @@ impl Tracer {
}

TraceResult {
source_map,
files: usages,
errors,
}
Expand Down
4 changes: 3 additions & 1 deletion crates/turborepo-lib/src/commands/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,9 @@ pub async fn run(
// If the arg starts with "query" or "mutation", and ends in a bracket, it's
// likely a direct query If it doesn't, it's a file path, so we need to
// read it
let query = if (trimmed_query.starts_with("query") || trimmed_query.starts_with("mutation"))
let query = if (trimmed_query.starts_with("query")
|| trimmed_query.starts_with("mutation")
|| trimmed_query.starts_with('{'))
&& trimmed_query.ends_with('}')
{
query
Expand Down
13 changes: 10 additions & 3 deletions crates/turborepo-lib/src/query/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::sync::Arc;
use async_graphql::{Enum, Object, SimpleObject};
use camino::Utf8PathBuf;
use itertools::Itertools;
use miette::SourceCode;
use swc_ecma_ast::EsVersion;
use swc_ecma_parser::{EsSyntax, Syntax, TsSyntax};
use turbo_trace::Tracer;
Expand Down Expand Up @@ -108,17 +109,21 @@ impl From<turbo_trace::TraceError> for TraceError {
message: format!("failed to glob files: {}", err),
..Default::default()
},
turbo_trace::TraceError::Resolve { span, text, .. } => {
turbo_trace::TraceError::Resolve {
span,
text,
file_path,
..
} => {
let import = text
.inner()
.read_span(&span, 1, 1)
.ok()
.map(|s| String::from_utf8_lossy(s.data()).to_string());

TraceError {
message,
import,
path: Some(text.name().to_string()),
path: Some(file_path),
start: Some(span.offset()),
end: Some(span.offset() + span.len()),
}
Expand Down Expand Up @@ -204,6 +209,7 @@ impl File {
}

let mut result = tracer.trace(depth).await;
result.emit_errors();
// Remove the file itself from the result
result.files.remove(&self.path);
TraceResult::new(result, self.run.clone())
Expand All @@ -225,6 +231,7 @@ impl File {
}

let mut result = tracer.reverse_trace().await;
result.emit_errors();
// Remove the file itself from the result
result.files.remove(&self.path);
TraceResult::new(result, self.run.clone())
Expand Down
2 changes: 1 addition & 1 deletion crates/turborepo/tests/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ fn test_trace() -> Result<(), anyhow::Error> {
"get `main.ts` with dependencies" => "query { file(path: \"main.ts\") { path, dependencies { files { items { path } } } } }",
"get `button.tsx` with dependencies" => "query { file(path: \"button.tsx\") { path, dependencies { files { items { path } } } } }",
"get `circular.ts` with dependencies" => "query { file(path: \"circular.ts\") { path dependencies { files { items { path } } } } }",
"get `invalid.ts` with dependencies" => "query { file(path: \"invalid.ts\") { path dependencies { files { items { path } } errors { items { message } } } } }",
"get `invalid.ts` with dependencies" => "query { file(path: \"invalid.ts\") { path dependencies { files { items { path } } errors { items { import } } } } }",
"get `main.ts` with depth = 0" => "query { file(path: \"main.ts\") { path dependencies(depth: 1) { files { items { path } } } } }",
"get `with_prefix.ts` with dependencies" => "query { file(path: \"with_prefix.ts\") { path dependencies { files { items { path } } } } }",
"get `import_value_and_type.ts` with all dependencies" => "query { file(path: \"import_value_and_type.ts\") { path dependencies(importType: ALL) { files { items { path } } } } }",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ expression: query_output
"errors": {
"items": [
{
"message": "failed to resolve import to `./non-existent-file.js`"
"import": "import foo from \"./non-existent-file.js\";\nimport { Button } from \"./button.tsx\";\n"
}
]
}
Expand Down
5 changes: 2 additions & 3 deletions turborepo-tests/integration/tests/turbo-trace.t
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,7 @@ Setup
}
Trace file with invalid import
$ ${TURBO} query "query { file(path: \"invalid.ts\") { path dependencies { files { items { path } } errors { items { message } } } } }"
WARNING query command is experimental and may change in the future
$ ${TURBO} query "query { file(path: \"invalid.ts\") { path dependencies { files { items { path } } errors { items { message } } } } }" 2>/dev/null
{
"data": {
"file": {
Expand All @@ -110,7 +109,7 @@ Trace file with invalid import
"errors": {
"items": [
{
"message": "failed to resolve import to `./non-existent-file.js`"
"message": "failed to resolve import to `./non-existent-file.js` in `.*`" (re)
}
]
}
Expand Down

0 comments on commit baa4eb5

Please sign in to comment.