denoland / deno_core

The core engine at the heart of Deno
MIT License
306 stars 98 forks source link

Empty `source_line` on JsError, after upgrading deno_core #961

Open liorcode opened 2 weeks ago

liorcode commented 2 weeks ago

I am using deno_core to create a Js runtime (following the great https://deno.com/blog/roll-your-own-javascript-runtime blog post)

Whenever I get an error evaluating some js source, I throw the JsError and report the bad line and code, which I take from the error source_line property.

After upgrading deno_core from 0.294 to the latest (0.319), I've noticed the source_line is always None.

Example:

Evaluating this broken code: console.log("test" would lead to this error:

JsError { name: Some("SyntaxError"), message: Some("missing ) after argument list"), stack: None, cause: None, exception_message: "Uncaught SyntaxError: missing ) after argument list", frames: [JsStackFrame { type_name: None, function_name: None, method_name: None, file_name: Some("test1"), line_number: Some(1), column_number: Some(13), ... }], source_line: Some("console.log(\"test\""), source_line_frame_index: Some(0), aggregated: None }

and after the upgrade, it leads to this error:

JsError { name: Some("SyntaxError"), message: Some("missing ) after argument list"), stack: None, cause: None, exception_message: "Uncaught SyntaxError: missing ) after argument list", frames: [JsStackFrame { type_name: None, function_name: None, method_name: None, file_name: Some("test1"), line_number: Some(1), column_number: Some(13), ... }], source_line: None, source_line_frame_index: Some(0), aggregated: None }

(notice the None source_line)

Some findings:

I tried bisecting to find where this problem first occurred, and it seems somewhere between 0.294 and 0.298 (not sure the exact version, since some of them didn't work at all for me) I compared the versions: https://github.com/denoland/deno_core/compare/0.294.0...0.298.0

and I found this change in Error trace, which seems related: https://github.com/denoland/deno_core/pull/827

specifically, the change in error.rs: https://github.com/denoland/deno_core/pull/827/files#diff-c8dcedfc29d534d8c70ded455f342bedc1611fb609bc5e62a379004fb496cc65

Perhaps removing the "fallback" behavior in the else is the cause for the missing source_line @bartlomieju? (the `source_line = msg.get_source_line(scope) part)

bartlomieju commented 2 weeks ago

IIRC the idea was that SourceMapGetter was supposed to always return the source line. Do you use one in your code?

liorcode commented 2 weeks ago

I don't think I use one. I don't even use a module loader - I just evaluate the code in something similar to:

let mut rt = JsRuntimeForSnapshot::new(RuntimeOptions { ..Default::default() });
let code = ModuleCodeString::from(script.code);
rt.execute_script(file_name, code)?;

where code, in the example is console.log("test"