denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
96.66k stars 5.34k forks source link

Reference Error in SetTimeout Task Can Crash #17743

Open waugh opened 1 year ago

waugh commented 1 year ago
$ deno
Deno 1.30.3
exit using ctrl+d, ctrl+c, or close()
REPL is running with all permissions allowed.
To specify permissions, run `deno repl` with allow flags.
> dogshit
Uncaught ReferenceError: dogshit is not defined
    at <anonymous>:2:1
> setTimeout(() => dogshit)
error: {"code":-32000,"message":"Execution was terminated"}
$
kamilogorek commented 1 year ago

The execution is terminated due to this line: https://github.com/denoland/deno/blob/d4e5a295f2f9af1f815596656a185b11d7dabb29/core/ops_builtin_v8.rs#L786-L792

However simply removing it won't cut it, as in REPL there's no error event handler, and https://github.com/denoland/deno/blob/d4e5a295f2f9af1f815596656a185b11d7dabb29/ext/web/02_event.js#L1490-L1492 will swallow the error.

Is there a reliable way to detect the REPL inside js runtime? If so, we could do the same thing that promiseRejectMacrotaskCallback already does, which is:

globalThis_.addEventListener("error", errorEventCb);
globalThis_.dispatchEvent(rejectionEvent);
globalThis_.removeEventListener("error", errorEventCb);

but moved to the initializeTimer, which is responsible for executing the task.

Related: https://github.com/denoland/deno/blob/main/cli/tests/integration/repl_tests.rs#L861

aapoalas commented 1 year ago

Is this related? https://github.com/denoland/deno/pull/16944

kamilogorek commented 1 year ago

Yup, that should fix it.