Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(runtime/js/workers): throw errors instead of using an op #12249

Merged
merged 4 commits into from Oct 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 14 additions & 0 deletions cli/tests/integration/worker_tests.rs
Expand Up @@ -20,6 +20,20 @@ itest!(worker_nested_error {
exit_code: 1,
});

itest!(worker_async_error {
args: "run -A --quiet --reload workers/worker_async_error.ts",
output: "workers/worker_async_error.ts.out",
http_server: true,
exit_code: 1,
});

itest!(worker_message_handler_error {
args: "run -A --quiet --reload workers/worker_message_handler_error.ts",
output: "workers/worker_message_handler_error.ts.out",
http_server: true,
exit_code: 1,
});

itest!(nonexistent_worker {
args: "run --allow-read workers/nonexistent_worker.ts",
output: "workers/nonexistent_worker.out",
Expand Down
2 changes: 1 addition & 1 deletion cli/tests/testdata/worker_drop_handle_race.js.out
Expand Up @@ -4,5 +4,5 @@ error: Uncaught (in worker "") Error
at [WILDCARD]/workers/drop_handle_race.js:2:9
at fire (deno:ext/timers/[WILDCARD])
at handleTimerMacrotask (deno:ext/timers/[WILDCARD])
error: Uncaught (in promise) Error: Unhandled error event reached main worker.
error: Uncaught (in promise) Error: Unhandled error event in child worker.
at Worker.#pollControl (deno:runtime/js/11_workers.js:[WILDCARD])
4 changes: 4 additions & 0 deletions cli/tests/testdata/workers/async_error.ts
@@ -0,0 +1,4 @@
// deno-lint-ignore require-await
(async () => {
throw new Error("bar");
})();
4 changes: 4 additions & 0 deletions cli/tests/testdata/workers/message_handler_error.ts
@@ -0,0 +1,4 @@
postMessage("ready");
onmessage = () => {
throw new Error("bar");
};
2 changes: 1 addition & 1 deletion cli/tests/testdata/workers/nonexistent_worker.out
@@ -1,3 +1,3 @@
[WILDCARD]error: Uncaught (in worker "") Cannot resolve module "file:///[WILDCARD]/workers/doesnt_exist.js".
error: Uncaught (in promise) Error: Unhandled error event reached main worker.
error: Uncaught (in promise) Error: Unhandled error event in child worker.
at Worker.#pollControl ([WILDCARD])
2 changes: 1 addition & 1 deletion cli/tests/testdata/workers/permissions_blob_local.ts.out
@@ -1,4 +1,4 @@
error: Uncaught (in worker "") Requires read access to "[WILDCARD]local_file.ts", run again with the --allow-read flag
at blob:null/[WILDCARD]:1:0
error: Uncaught (in promise) Error: Unhandled error event reached main worker.
error: Uncaught (in promise) Error: Unhandled error event in child worker.
at Worker.#pollControl ([WILDCARD])
2 changes: 1 addition & 1 deletion cli/tests/testdata/workers/permissions_blob_remote.ts.out
@@ -1,4 +1,4 @@
error: Uncaught (in worker "") Requires net access to "example.com", run again with the --allow-net flag
at blob:null/[WILDCARD]:1:0
error: Uncaught (in promise) Error: Unhandled error event reached main worker.
error: Uncaught (in promise) Error: Unhandled error event in child worker.
at Worker.#pollControl ([WILDCARD])
2 changes: 1 addition & 1 deletion cli/tests/testdata/workers/permissions_data_local.ts.out
@@ -1,4 +1,4 @@
error: Uncaught (in worker "") Requires read access to "[WILDCARD]local_file.ts", run again with the --allow-read flag
at data:application/javascript;base64,[WILDCARD]:1:0
error: Uncaught (in promise) Error: Unhandled error event reached main worker.
error: Uncaught (in promise) Error: Unhandled error event in child worker.
at Worker.#pollControl ([WILDCARD])
2 changes: 1 addition & 1 deletion cli/tests/testdata/workers/permissions_data_remote.ts.out
@@ -1,4 +1,4 @@
error: Uncaught (in worker "") Requires net access to "example.com", run again with the --allow-net flag
at data:application/javascript;base64,aW1wb3J0ICJodHRwczovL2V4YW1wbGUuY29tL3NvbWUvZmlsZS50cyI7:1:0
error: Uncaught (in promise) Error: Unhandled error event reached main worker.
error: Uncaught (in promise) Error: Unhandled error event in child worker.
at Worker.#pollControl ([WILDCARD])
Expand Up @@ -2,5 +2,5 @@ error: Uncaught (in worker "") (in promise) TypeError: Requires net access to "e
await import("https://example.com/some/file.ts");
^
at async http://localhost:4545/workers/dynamic_remote.ts:2:1
[WILDCARD]error: Uncaught (in promise) Error: Unhandled error event reached main worker.
[WILDCARD]error: Uncaught (in promise) Error: Unhandled error event in child worker.
at Worker.#pollControl ([WILDCARD])
@@ -1,4 +1,4 @@
error: Uncaught (in worker "") Requires net access to "example.com", run again with the --allow-net flag
at http://localhost:4545/workers/static_remote.ts:2:0
error: Uncaught (in promise) Error: Unhandled error event reached main worker.
error: Uncaught (in promise) Error: Unhandled error event in child worker.
at Worker.#pollControl ([WILDCARD])
5 changes: 0 additions & 5 deletions cli/tests/testdata/workers/test.ts
Expand Up @@ -34,11 +34,6 @@ Deno.test({
tsWorker.postMessage("Hello World");
};

jsWorker.onerror = (e: Event) => {
e.preventDefault();
jsWorker.postMessage("Hello World");
};

jsWorker.postMessage("Hello World");
await promise;
tsWorker.terminate();
Expand Down
11 changes: 0 additions & 11 deletions cli/tests/testdata/workers/test_worker.js
@@ -1,19 +1,8 @@
let thrown = false;

if (self.name !== "") {
throw Error(`Bad worker name: ${self.name}, expected empty string.`);
}

onmessage = function (e) {
if (thrown === false) {
thrown = true;
throw new SyntaxError("[test error]");
}

postMessage(e.data);
close();
};

onerror = function () {
return false;
};
5 changes: 5 additions & 0 deletions cli/tests/testdata/workers/worker_async_error.ts
@@ -0,0 +1,5 @@
const worker = new Worker(
new URL("async_error.ts", import.meta.url).href,
{ type: "module", name: "foo" },
);
setTimeout(() => worker.terminate(), 30000);
7 changes: 7 additions & 0 deletions cli/tests/testdata/workers/worker_async_error.ts.out
@@ -0,0 +1,7 @@
error: Uncaught (in worker "foo") (in promise) Error: bar
throw new Error("bar");
^
at [WILDCARD]/async_error.ts:[WILDCARD]
at [WILDCARD]/async_error.ts:[WILDCARD]
error: Uncaught (in promise) Error: Unhandled error event in child worker.
at Worker.#pollControl ([WILDCARD])
2 changes: 1 addition & 1 deletion cli/tests/testdata/workers/worker_error.ts.out
@@ -1,5 +1,5 @@
[WILDCARD]error: Uncaught (in worker "bar") Error: foo[WILDCARD]
at foo ([WILDCARD])
at [WILDCARD]
error: Uncaught (in promise) Error: Unhandled error event reached main worker.
error: Uncaught (in promise) Error: Unhandled error event in child worker.
at Worker.#pollControl ([WILDCARD])
8 changes: 8 additions & 0 deletions cli/tests/testdata/workers/worker_message_handler_error.ts
@@ -0,0 +1,8 @@
const worker = new Worker(
new URL("message_handler_error.ts", import.meta.url).href,
{ type: "module", name: "foo" },
);
worker.onmessage = () => {
worker.postMessage("ready");
};
setTimeout(() => worker.terminate(), 30000);
@@ -0,0 +1,7 @@
error: Uncaught (in worker "foo") (in promise) Error: bar
throw new Error("bar");
^
at onmessage ([WILDCARD]/message_handler_error.ts:[WILDCARD])
at [WILDCARD]
error: Uncaught (in promise) Error: Unhandled error event in child worker.
at Worker.#pollControl ([WILDCARD])
10 changes: 7 additions & 3 deletions cli/tests/testdata/workers/worker_nested_error.ts.out
@@ -1,5 +1,9 @@
[WILDCARD]error: Uncaught (in worker "bar") Error: foo[WILDCARD]
at foo ([WILDCARD])
at [WILDCARD]
error: Uncaught (in promise) Error: Unhandled error event reached main worker.
throw new Error("foo");
^
at foo ([WILDCARD]/workers/error.ts:[WILDCARD])
at [WILDCARD]/workers/error.ts:[WILDCARD]
error: Uncaught (in worker "baz") (in promise) Error: Unhandled error event in child worker.
at Worker.#pollControl ([WILDCARD])
error: Uncaught (in promise) Error: Unhandled error event in child worker.
at Worker.#pollControl ([WILDCARD])
10 changes: 1 addition & 9 deletions runtime/js/11_workers.js
Expand Up @@ -14,7 +14,6 @@
} = window.__bootstrap.primordials;
const webidl = window.__bootstrap.webidl;
const { URL } = window.__bootstrap.url;
const { Window } = window.__bootstrap.globalInterfaces;
const { getLocationHref } = window.__bootstrap.location;
const { log, pathFromURL } = window.__bootstrap.util;
const { defineEventHandler } = window.__bootstrap.webUtil;
Expand Down Expand Up @@ -264,14 +263,7 @@
} /* falls through */
case 2: { // Error
if (!this.#handleError(data)) {
if (globalThis instanceof Window) {
throw new Error("Unhandled error event reached main worker.");
} else {
core.opSync(
"op_worker_unhandled_error",
data.message,
);
}
throw new Error("Unhandled error event in child worker.");
}
break;
}
Expand Down
5 changes: 1 addition & 4 deletions runtime/js/99_main.js
Expand Up @@ -157,10 +157,7 @@ delete Object.prototype.__proto__;

globalDispatchEvent(errorEvent);
if (!errorEvent.defaultPrevented) {
core.opSync(
"op_worker_unhandled_error",
e.message,
);
throw e;
}
}
}
Expand Down
24 changes: 0 additions & 24 deletions runtime/ops/web_worker.rs
Expand Up @@ -4,8 +4,6 @@ mod sync_fetch;

use crate::web_worker::WebWorkerInternalHandle;
use crate::web_worker::WebWorkerType;
use crate::web_worker::WorkerControlEvent;
use deno_core::error::generic_error;
use deno_core::error::AnyError;
use deno_core::op_async;
use deno_core::op_sync;
Expand All @@ -25,11 +23,6 @@ pub fn init() -> Extension {
("op_worker_recv_message", op_async(op_worker_recv_message)),
// Notify host that guest worker closes.
("op_worker_close", op_sync(op_worker_close)),
// Notify host that guest worker has unhandled error.
(
"op_worker_unhandled_error",
op_sync(op_worker_unhandled_error),
),
("op_worker_get_type", op_sync(op_worker_get_type)),
("op_worker_sync_fetch", op_sync(op_worker_sync_fetch)),
])
Expand Down Expand Up @@ -70,23 +63,6 @@ fn op_worker_close(state: &mut OpState, _: (), _: ()) -> Result<(), AnyError> {
Ok(())
}

/// A worker that encounters an uncaught error will pass this error
/// to its parent worker using this op. The parent worker will use
/// this same op to pass the error to its own parent (in case
/// `e.preventDefault()` was not called in `worker.onerror`). This
/// is done until the error reaches the root/ main worker.
fn op_worker_unhandled_error(
state: &mut OpState,
message: String,
_: (),
) -> Result<(), AnyError> {
let sender = state.borrow::<WebWorkerInternalHandle>().clone();
sender
.post_event(WorkerControlEvent::Error(generic_error(message)))
.expect("Failed to propagate error event to parent worker");
Ok(())
}

fn op_worker_get_type(
state: &mut OpState,
_: (),
Expand Down
16 changes: 7 additions & 9 deletions runtime/web_worker.rs
Expand Up @@ -522,15 +522,8 @@ impl WebWorker {
return Poll::Ready(Ok(()));
}

// In case of an error, pass to parent without terminating worker
if let Err(e) = r {
print_worker_error(e.to_string(), &self.name);
let handle = self.internal_handle.clone();
handle
.post_event(WorkerControlEvent::Error(e))
.expect("Failed to post message to host");

return Poll::Pending;
return Poll::Ready(Err(e));
}

panic!(
Expand Down Expand Up @@ -590,6 +583,12 @@ pub fn run_web_worker(
return Ok(());
}

let result = if result.is_ok() {
worker.run_event_loop(true).await
} else {
result
};

if let Err(e) = result {
print_worker_error(e.to_string(), &name);
internal_handle
Expand All @@ -600,7 +599,6 @@ pub fn run_web_worker(
return Ok(());
}

let result = worker.run_event_loop(true).await;
debug!("Worker thread shuts down {}", &name);
result
};
Expand Down