Skip to content

Commit

Permalink
Restore Issue Reporters (vercel/turborepo#3803)
Browse files Browse the repository at this point in the history
This restores issue reporters, addressing a bug that prevented turbo-trace from completing. This moves `ConsoleUiVc::new` into an async block to prevent it from stalling.

Test Plan: Verify `cargo run --bin node-file-trace -- print path/to/my/app` no longer stalls.
  • Loading branch information
wbinnssmith authored Feb 14, 2023
1 parent 892d295 commit e191b66
Show file tree
Hide file tree
Showing 9 changed files with 168 additions and 107 deletions.
85 changes: 61 additions & 24 deletions crates/next-dev/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,15 @@ use owo_colors::OwoColorize;
use turbo_malloc::TurboMalloc;
use turbo_tasks::{
util::{FormatBytes, FormatDuration},
RawVc, StatsType, TransientInstance, TransientValue, TurboTasks, TurboTasksBackendApi, Value,
CollectiblesSource, RawVc, StatsType, TransientInstance, TransientValue, TurboTasks,
TurboTasksBackendApi, Value,
};
use turbo_tasks_fs::{DiskFileSystemVc, FileSystem, FileSystemVc};
use turbo_tasks_memory::MemoryBackend;
use turbopack_cli_utils::issue::{ConsoleUi, ConsoleUiVc, LogOptions};
use turbopack_cli_utils::issue::{ConsoleUiVc, LogOptions};
use turbopack_core::{
environment::ServerAddr,
issue::IssueSeverity,
issue::{IssueReporter, IssueReporterVc, IssueSeverity, IssueVc},
resolve::{parse::RequestVc, pattern::QueryMapVc},
server_fs::ServerFileSystemVc,
};
Expand Down Expand Up @@ -62,6 +63,7 @@ pub struct NextDevServerBuilder {
entry_requests: Vec<EntryRequest>,
eager_compile: bool,
hostname: Option<IpAddr>,
issue_reporter: Option<Box<dyn IssueReporterProvider>>,
port: Option<u16>,
browserslist_query: String,
log_level: IssueSeverity,
Expand All @@ -83,6 +85,7 @@ impl NextDevServerBuilder {
entry_requests: vec![],
eager_compile: false,
hostname: None,
issue_reporter: None,
port: None,
browserslist_query: "last 1 Chrome versions, last 1 Firefox versions, last 1 Safari \
versions, last 1 Edge versions"
Expand Down Expand Up @@ -139,6 +142,14 @@ impl NextDevServerBuilder {
self
}

pub fn issue_reporter(
mut self,
issue_reporter: Box<dyn IssueReporterProvider>,
) -> NextDevServerBuilder {
self.issue_reporter = Some(issue_reporter);
self
}

/// Attempts to find an open port to bind.
fn find_port(&self, host: IpAddr, port: u16, max_attempts: u16) -> Result<DevServerBuilder> {
// max_attempts of 1 means we loop 0 times.
Expand Down Expand Up @@ -192,58 +203,72 @@ impl NextDevServerBuilder {
let show_all = self.show_all;
let log_detail = self.log_detail;
let browserslist_query = self.browserslist_query;
let log_options = LogOptions {
let log_options = Arc::new(LogOptions {
current_dir: current_dir().unwrap(),
show_all,
log_detail,
log_level: self.log_level,
};
});
let entry_requests = Arc::new(self.entry_requests);
let console_ui = Arc::new(ConsoleUi::new(log_options));
let console_ui_to_dev_server = console_ui.clone();
let server_addr = Arc::new(server.addr);
let tasks = turbo_tasks.clone();
let issue_provider = self.issue_reporter.unwrap_or_else(|| {
// Initialize a ConsoleUi reporter if no custom reporter was provided
Box::new(move || ConsoleUiVc::new(log_options.clone().into()).into())
});
let issue_reporter_arc = Arc::new(move || issue_provider.get_issue_reporter());

let get_issue_reporter = issue_reporter_arc.clone();
let source = move || {
source(
root_dir.clone(),
project_dir.clone(),
entry_requests.clone().into(),
eager_compile,
turbo_tasks.clone().into(),
console_ui.clone().into(),
get_issue_reporter(),
browserslist_query.clone(),
server_addr.clone().into(),
)
};

Ok(server.serve(tasks, source, console_ui_to_dev_server))
Ok(server.serve(tasks, source, issue_reporter_arc.clone()))
}
}

async fn handle_issues<T: Into<RawVc>>(source: T, console_ui: ConsoleUiVc) -> Result<()> {
let state = console_ui
.group_and_display_issues(TransientValue::new(source.into()))
async fn handle_issues<T: Into<RawVc> + CollectiblesSource + Copy>(
source: T,
issue_reporter: IssueReporterVc,
) -> Result<()> {
let issues = IssueVc::peek_issues_with_path(source)
.await?
.strongly_consistent()
.await?;

if state.has_fatal {
issue_reporter.report_issues(
TransientInstance::new(issues.clone()),
TransientValue::new(source.into()),
);

if issues.has_fatal().await? {
Err(anyhow!("Fatal issue(s) occurred"))
} else {
Ok(())
}
}

#[turbo_tasks::function]
async fn project_fs(project_dir: &str, console_ui: ConsoleUiVc) -> Result<FileSystemVc> {
async fn project_fs(project_dir: &str, issue_reporter: IssueReporterVc) -> Result<FileSystemVc> {
let disk_fs = DiskFileSystemVc::new("project".to_string(), project_dir.to_string());
handle_issues(disk_fs, console_ui).await?;
handle_issues(disk_fs, issue_reporter).await?;
disk_fs.await?.start_watching()?;
Ok(disk_fs.into())
}

#[turbo_tasks::function]
async fn output_fs(project_dir: &str, console_ui: ConsoleUiVc) -> Result<FileSystemVc> {
async fn output_fs(project_dir: &str, issue_reporter: IssueReporterVc) -> Result<FileSystemVc> {
let disk_fs = DiskFileSystemVc::new("output".to_string(), project_dir.to_string());
handle_issues(disk_fs, console_ui).await?;
handle_issues(disk_fs, issue_reporter).await?;
disk_fs.await?.start_watching()?;
Ok(disk_fs.into())
}
Expand All @@ -256,13 +281,12 @@ async fn source(
entry_requests: TransientInstance<Vec<EntryRequest>>,
eager_compile: bool,
turbo_tasks: TransientInstance<TurboTasks<MemoryBackend>>,
console_ui: TransientInstance<ConsoleUi>,
issue_reporter: IssueReporterVc,
browserslist_query: String,
server_addr: TransientInstance<SocketAddr>,
) -> Result<ContentSourceVc> {
let console_ui = (*console_ui).clone().cell();
let output_fs = output_fs(&project_dir, console_ui);
let fs = project_fs(&root_dir, console_ui);
let output_fs = output_fs(&project_dir, issue_reporter);
let fs = project_fs(&root_dir, issue_reporter);
let project_relative = project_dir.strip_prefix(&root_dir).unwrap();
let project_relative = project_relative
.strip_prefix(MAIN_SEPARATOR)
Expand Down Expand Up @@ -372,9 +396,9 @@ async fn source(
.cell()
.into();

handle_issues(dev_server_fs, console_ui).await?;
handle_issues(web_source, console_ui).await?;
handle_issues(page_source, console_ui).await?;
handle_issues(dev_server_fs, issue_reporter).await?;
handle_issues(web_source, issue_reporter).await?;
handle_issues(page_source, issue_reporter).await?;

Ok(source)
}
Expand Down Expand Up @@ -551,3 +575,16 @@ fn profile_timeout<T>(
) -> impl Future<Output = T> {
future
}

pub trait IssueReporterProvider: Send + Sync + 'static {
fn get_issue_reporter(&self) -> IssueReporterVc;
}

impl<T> IssueReporterProvider for T
where
T: Fn() -> IssueReporterVc + Send + Sync + Clone + 'static,
{
fn get_issue_reporter(&self) -> IssueReporterVc {
self()
}
}
28 changes: 16 additions & 12 deletions crates/node-file-trace/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,12 @@ use turbopack::{
resolve_options_context::ResolveOptionsContext, transition::TransitionsByNameVc,
ModuleAssetContextVc,
};
use turbopack_cli_utils::issue::{ConsoleUi, IssueSeverityCliOption, LogOptions};
use turbopack_cli_utils::issue::{ConsoleUiVc, IssueSeverityCliOption, LogOptions};
use turbopack_core::{
asset::{Asset, AssetVc, AssetsVc},
context::{AssetContext, AssetContextVc},
environment::{EnvironmentIntention, EnvironmentVc, ExecutionEnvironment, NodeJsEnvironment},
issue::{IssueSeverity, IssueVc},
issue::{IssueReporter, IssueSeverity, IssueVc},
reference::all_assets,
resolve::options::{ImportMapping, ResolvedMap},
source_asset::SourceAssetVc,
Expand Down Expand Up @@ -487,25 +487,29 @@ async fn run<B: Backend + 'static, F: Future<Output = ()>>(
let (sender, mut receiver) = channel(1);
let dir = current_dir().unwrap();
let tt = create_tt();
let console_ui = Arc::new(ConsoleUi::new(LogOptions {
current_dir: dir.clone(),
show_all,
log_detail,
log_level: log_level.map_or_else(|| IssueSeverity::Error, |l| l.0),
}));
let task = tt.spawn_root_task(move || {
let dir = dir.clone();
let args = args.clone();
let console_ui = console_ui.clone();
let sender = sender.clone();
Box::pin(async move {
let output = main_operation(TransientValue::new(dir.clone()), args.clone().into());

let console_ui = (*console_ui).clone().cell();
console_ui
.group_and_display_issues(TransientValue::new(output.into()))
let source = TransientValue::new(output.into());
let issues = IssueVc::peek_issues_with_path(output)
.await?
.strongly_consistent()
.await?;

let console_ui = ConsoleUiVc::new(TransientInstance::new(LogOptions {
current_dir: dir.clone(),
show_all,
log_detail,
log_level: log_level.map_or_else(|| IssueSeverity::Error, |l| l.0),
}));
console_ui
.as_issue_reporter()
.report_issues(TransientInstance::new(issues), source);

if has_return_value {
let output_read_ref = output.await?;
let output_iter = output_read_ref.iter().cloned();
Expand Down
61 changes: 25 additions & 36 deletions crates/turbopack-cli-utils/src/issue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,17 @@ use std::{
use anyhow::{anyhow, Result};
use crossterm::style::{StyledContent, Stylize};
use owo_colors::{OwoColorize as _, Style};
use turbo_tasks::{RawVc, TransientValue, TryJoinIterExt, ValueToString};
use turbo_tasks::{
RawVc, ReadRef, TransientInstance, TransientValue, TryJoinIterExt, ValueToString,
};
use turbo_tasks_fs::{
attach::AttachedFileSystemVc,
source_context::{get_source_context, SourceContextLine},
to_sys_path, FileLinesContent, FileSystemPathVc,
};
use turbopack_core::issue::{
Issue, IssueProcessingPathItem, IssueSeverity, IssueVc, OptionIssueProcessingPathItemsVc,
PlainIssue, PlainIssueSource,
CapturedIssues, Issue, IssueProcessingPathItem, IssueReporter, IssueReporterVc, IssueSeverity,
OptionIssueProcessingPathItemsVc, PlainIssue, PlainIssueSource,
};

#[derive(Clone, Copy, PartialEq, Eq, Debug)]
Expand Down Expand Up @@ -412,41 +414,34 @@ impl PartialEq for ConsoleUi {
}
}

impl ConsoleUi {
pub fn new(options: LogOptions) -> Self {
#[turbo_tasks::value_impl]
impl ConsoleUiVc {
#[turbo_tasks::function]
pub fn new(options: TransientInstance<LogOptions>) -> Self {
ConsoleUi {
options,
options: (*options).clone(),
seen: Arc::new(Mutex::new(SeenIssues::new())),
}
.cell()
}
}

#[turbo_tasks::value(transparent)]
pub struct DisplayIssueState {
pub has_fatal: bool,
pub has_issues: bool,
pub has_new_issues: bool,
}

#[turbo_tasks::value_impl]
impl ConsoleUiVc {
impl IssueReporter for ConsoleUi {
#[turbo_tasks::function]
pub async fn group_and_display_issues(
self,
async fn report_issues(
&self,
issues: TransientInstance<ReadRef<CapturedIssues>>,
source: TransientValue<RawVc>,
) -> Result<DisplayIssueStateVc> {
let source = source.into_value();
let this = self.await?;

let issues = IssueVc::peek_issues_with_path(source).await?;
let issues = issues.await?;
let &LogOptions {
) -> Result<()> {
let issues = &*issues;
let LogOptions {
ref current_dir,
show_all,
log_detail,
log_level,
..
} = &this.options;
} = self.options;
let mut grouped_issues: GroupedIssues = HashMap::new();

let issues = issues
Expand All @@ -464,11 +459,11 @@ impl ConsoleUiVc {
.iter()
.map(|(_, _, _, id)| *id)
.collect::<HashSet<_>>();
let mut new_ids = this.seen.lock().unwrap().new_ids(source, issue_ids);

let mut has_fatal = false;
let has_issues = !issues.is_empty();
let has_new_issues = !new_ids.is_empty();
let mut new_ids = self
.seen
.lock()
.unwrap()
.new_ids(source.into_value(), issue_ids);

for (plain_issue, path, context, id) in issues {
if !new_ids.remove(&id) {
Expand All @@ -479,7 +474,6 @@ impl ConsoleUiVc {
let context_path = make_relative_to_cwd(context, current_dir).await?;
let category = &plain_issue.category;
let title = &plain_issue.title;
has_fatal = severity == IssueSeverity::Fatal;
let severity_map = grouped_issues
.entry(severity)
.or_insert_with(Default::default);
Expand Down Expand Up @@ -612,12 +606,7 @@ impl ConsoleUiVc {
}
}

Ok(DisplayIssueState {
has_fatal,
has_issues,
has_new_issues,
}
.cell())
Ok(())
}
}

Expand Down
27 changes: 26 additions & 1 deletion crates/turbopack-core/src/issue/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ use futures::FutureExt;
use turbo_tasks::{
emit,
primitives::{BoolVc, StringVc, U64Vc},
CollectiblesSource, ReadRef, TryJoinIterExt, ValueToString, ValueToStringVc,
CollectiblesSource, RawVc, ReadRef, TransientInstance, TransientValue, TryJoinIterExt,
ValueToString, ValueToStringVc,
};
use turbo_tasks_fs::{
FileContent, FileContentReadRef, FileLine, FileLinesContent, FileSystemPathReadRef,
Expand Down Expand Up @@ -340,6 +341,21 @@ pub struct CapturedIssues {
processing_path: ItemIssueProcessingPathVc,
}

impl CapturedIssues {
pub async fn has_fatal(&self) -> Result<bool> {
let mut has_fatal = false;

for issue in self.issues.iter() {
let severity = *issue.severity().await?;
if severity == IssueSeverity::Fatal {
has_fatal = true;
break;
}
}
Ok(has_fatal)
}
}

#[turbo_tasks::value_impl]
impl CapturedIssuesVc {
#[turbo_tasks::function]
Expand Down Expand Up @@ -568,3 +584,12 @@ impl PlainAssetVc {
.cell())
}
}

#[turbo_tasks::value_trait]
pub trait IssueReporter {
fn report_issues(
&self,
issues: TransientInstance<ReadRef<CapturedIssues>>,
source: TransientValue<RawVc>,
);
}
Loading

0 comments on commit e191b66

Please sign in to comment.