Skip to content

Commit

Permalink
fix(cli/fmt_errors): don't panic on source line formatting errors (de…
Browse files Browse the repository at this point in the history
…noland#12449)

Returns empty values in case of errors, source lines are non-essential anyway. These errors can happen e.g. when source files change at runtime. A warning is also printed to help us track when it happens in unexpected cases besides this.
  • Loading branch information
nayeemrmn committed Oct 18, 2021
1 parent 0a7ba33 commit 5a48d41
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 2 deletions.
11 changes: 11 additions & 0 deletions cli/fmt_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,12 +177,23 @@ fn format_maybe_source_line(
if source_line.is_empty() || source_line.len() > SOURCE_ABBREV_THRESHOLD {
return "".to_string();
}
if source_line.contains("Couldn't format source line: ") {
return format!("\n{}", source_line);
}

assert!(start_column.is_some());
assert!(end_column.is_some());
let mut s = String::new();
let start_column = start_column.unwrap();
let end_column = end_column.unwrap();

if start_column as usize >= source_line.len() {
return format!(
"\n{} Couldn't format source line: Column {} is out of bounds (source may have changed at runtime)",
crate::colors::yellow("Warning"), start_column + 1,
);
}

// TypeScript uses `~` always, but V8 would utilise `^` always, even when
// doing ranges, so here, if we only have one marker (very common with V8
// errors) we will use `^` instead.
Expand Down
10 changes: 8 additions & 2 deletions cli/proc_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -622,8 +622,14 @@ impl SourceMapGetter for ProcState {
// Do NOT use .lines(): it skips the terminating empty line.
// (due to internally using .split_terminator() instead of .split())
let lines: Vec<&str> = out.source.split('\n').collect();
assert!(lines.len() > line_number);
lines[line_number].to_string()
if line_number >= lines.len() {
format!(
"{} Couldn't format source line: Line {} is out of bounds (source may have changed at runtime)",
crate::colors::yellow("Warning"), line_number + 1,
)
} else {
lines[line_number].to_string()
}
})
} else {
None
Expand Down
6 changes: 6 additions & 0 deletions cli/tests/integration/run_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1916,3 +1916,9 @@ itest!(long_data_url_formatting {
output: "long_data_url_formatting.ts.out",
exit_code: 1,
});

itest!(eval_context_throw_with_conflicting_source {
args: "run eval_context_throw_with_conflicting_source.ts",
output: "eval_context_throw_with_conflicting_source.ts.out",
exit_code: 1,
});
1 change: 1 addition & 0 deletions cli/tests/testdata/eval_context_conflicting_source.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
throw new Error("foo");
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// deno-lint-ignore no-explicit-any
const [, errorInfo] = (Deno as any).core.evalContext(
'/* aaaaaaaaaaaaaaaaa */ throw new Error("foo")',
new URL("eval_context_conflicting_source.ts", import.meta.url).href,
);
throw errorInfo.thrown;
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
[WILDCARD]error: Uncaught Error: foo
Warning Couldn't format source line: Column 31 is out of bounds (source may have changed at runtime)
at file:https:///[WILDCARD]/eval_context_conflicting_source.ts:1:31
at file:https:///[WILDCARD]/eval_context_throw_with_conflicting_source.ts:[WILDCARD]

0 comments on commit 5a48d41

Please sign in to comment.