From 3e7afb8918fd0f6cedf839a7ebaae6aaee5e66ad Mon Sep 17 00:00:00 2001 From: Andreu Botella Date: Fri, 13 May 2022 10:36:31 +0200 Subject: [PATCH] chore(runtime): Make some ops in ext and runtime infallible. (#14589) Co-authored-by: Aaron O'Mullan --- bench_util/benches/op_baseline.rs | 18 ++++---------- cli/build.rs | 17 ++++++------- cli/lsp/tsc.rs | 41 +++++++++++-------------------- cli/ops/bench.rs | 14 +++-------- cli/tsc.rs | 40 +++++++++++++++--------------- core/modules.rs | 5 ++-- ext/url/lib.rs | 6 ++--- ext/web/blob.rs | 11 +++------ ext/web/compression.rs | 4 +-- ext/web/lib.rs | 19 +++++++------- ext/web/message_port.rs | 4 +-- ext/web/timers.rs | 19 +++++--------- runtime/ops/os.rs | 5 ++-- runtime/ops/web_worker.rs | 7 +++--- runtime/ops/worker_host.rs | 6 +---- 15 files changed, 83 insertions(+), 133 deletions(-) diff --git a/bench_util/benches/op_baseline.rs b/bench_util/benches/op_baseline.rs index 052ac2cb23..46501e4f73 100644 --- a/bench_util/benches/op_baseline.rs +++ b/bench_util/benches/op_baseline.rs @@ -2,14 +2,8 @@ use deno_bench_util::bench_or_profile; use deno_bench_util::bencher::{benchmark_group, Bencher}; use deno_bench_util::{bench_js_async, bench_js_sync}; -use deno_core::error::AnyError; use deno_core::op; - use deno_core::Extension; -use deno_core::OpState; - -use std::cell::RefCell; -use std::rc::Rc; fn setup() -> Vec { vec![Extension::builder() @@ -22,19 +16,17 @@ fn setup() -> Vec { } #[op] -fn op_nop() -> Result<(), AnyError> { - Ok(()) -} +fn op_nop() {} #[op] -fn op_pi_json() -> Result { - Ok(314159) +fn op_pi_json() -> i64 { + 314159 } // this is a function since async closures aren't stable #[op] -async fn op_pi_async(_: Rc>) -> Result { - Ok(314159) +async fn op_pi_async() -> i64 { + 314159 } fn bench_op_pi_json(b: &mut Bencher) { diff --git a/cli/build.rs b/cli/build.rs index 89afe642c3..019f8d02dd 100644 --- a/cli/build.rs +++ b/cli/build.rs @@ -178,26 +178,23 @@ fn create_compiler_snapshot( } #[op] - fn op_build_info( - state: &mut OpState, - _args: Value, - ) -> Result { + fn op_build_info(state: &mut OpState) -> Value { let build_specifier = "asset:///bootstrap.ts"; let build_libs = state.borrow::>(); - Ok(json!({ + json!({ "buildSpecifier": build_specifier, "libs": build_libs, - })) + }) } #[op] - fn op_cwd(_args: Value) -> Result { - Ok(json!("cache:///")) + fn op_cwd() -> String { + "cache:///".into() } #[op] - fn op_exists(_args: Value) -> Result { - Ok(json!(false)) + fn op_exists() -> bool { + false } #[op] diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 0557179b69..27d32b2dba 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -2489,10 +2489,7 @@ struct SpecifierArgs { } #[op] -fn op_exists( - state: &mut OpState, - args: SpecifierArgs, -) -> Result { +fn op_exists(state: &mut OpState, args: SpecifierArgs) -> bool { let state = state.borrow_mut::(); // we don't measure the performance of op_exists anymore because as of TS 4.5 // it is noisy with all the checking for custom libs, that we can't see the @@ -2504,10 +2501,9 @@ fn op_exists( // sometimes tsc tries to query invalid specifiers, especially when // something else isn't quite right, so instead of bubbling up the error // back to tsc, we simply swallow it and say the file doesn't exist - Err(_) => return Ok(false), + Err(_) => return false, }; - let result = state.state_snapshot.documents.exists(&specifier); - Ok(result) + state.state_snapshot.documents.exists(&specifier) } #[derive(Debug, Deserialize, Serialize)] @@ -2621,9 +2617,9 @@ fn op_get_text( } #[op] -fn op_is_cancelled(state: &mut OpState) -> Result { +fn op_is_cancelled(state: &mut OpState) -> bool { let state = state.borrow_mut::(); - Ok(state.token.is_cancelled()) + state.token.is_cancelled() } #[op] @@ -2676,27 +2672,22 @@ fn op_resolve( } #[op] -fn op_respond(state: &mut OpState, args: Response) -> Result { +fn op_respond(state: &mut OpState, args: Response) -> bool { let state = state.borrow_mut::(); state.response = Some(args); - Ok(true) + true } #[op] -fn op_script_names( - state: &mut OpState, - _args: Value, -) -> Result, AnyError> { +fn op_script_names(state: &mut OpState) -> Vec { let state = state.borrow_mut::(); - Ok( - state - .state_snapshot - .documents - .documents(true, true) - .into_iter() - .map(|d| d.specifier().clone()) - .collect(), - ) + state + .state_snapshot + .documents + .documents(true, true) + .into_iter() + .map(|d| d.specifier().clone()) + .collect() } #[derive(Debug, Deserialize, Serialize)] @@ -3844,8 +3835,6 @@ mod tests { specifier: "/error/unknown:something/index.d.ts".to_string(), }, ); - assert!(actual.is_ok()); - let actual = actual.unwrap(); assert!(!actual); } diff --git a/cli/ops/bench.rs b/cli/ops/bench.rs index 6f4b80974e..e028aa6b1f 100644 --- a/cli/ops/bench.rs +++ b/cli/ops/bench.rs @@ -45,9 +45,8 @@ fn check_unstable(state: &OpState, api_name: &str) { } #[op] -fn op_bench_check_unstable(state: &mut OpState) -> Result<(), AnyError> { +fn op_bench_check_unstable(state: &mut OpState) { check_unstable(state, "Deno.bench"); - Ok(()) } #[derive(Clone)] @@ -94,19 +93,14 @@ pub fn op_restore_test_permissions( } #[op] -fn op_get_bench_origin(state: &mut OpState) -> Result { - Ok(state.borrow::().to_string()) +fn op_get_bench_origin(state: &mut OpState) -> String { + state.borrow::().to_string() } #[op] -fn op_dispatch_bench_event( - state: &mut OpState, - event: BenchEvent, -) -> Result<(), AnyError> { +fn op_dispatch_bench_event(state: &mut OpState, event: BenchEvent) { let sender = state.borrow::>().clone(); sender.send(event).ok(); - - Ok(()) } #[op] diff --git a/cli/tsc.rs b/cli/tsc.rs index c39ed6c0d0..aba289d8cc 100644 --- a/cli/tsc.rs +++ b/cli/tsc.rs @@ -344,7 +344,7 @@ struct EmitArgs { } #[op] -fn op_emit(state: &mut OpState, args: EmitArgs) -> Result { +fn op_emit(state: &mut OpState, args: EmitArgs) -> bool { let state = state.borrow_mut::(); match args.file_name.as_ref() { "deno:///.tsbuildinfo" => state.maybe_tsbuildinfo = Some(args.data), @@ -389,7 +389,7 @@ fn op_emit(state: &mut OpState, args: EmitArgs) -> Result { } } - Ok(json!(true)) + true } #[derive(Debug, Deserialize)] @@ -399,20 +399,20 @@ struct ExistsArgs { } #[op] -fn op_exists(state: &mut OpState, args: ExistsArgs) -> Result { +fn op_exists(state: &mut OpState, args: ExistsArgs) -> bool { let state = state.borrow_mut::(); let graph_data = state.graph_data.read(); if let Ok(specifier) = normalize_specifier(&args.specifier) { if specifier.scheme() == "asset" || specifier.scheme() == "data" { - Ok(true) + true } else { - Ok(matches!( + matches!( graph_data.get(&graph_data.follow_redirect(&specifier)), Some(ModuleEntry::Module { .. }) - )) + ) } } else { - Ok(false) + false } } @@ -510,7 +510,7 @@ pub struct ResolveArgs { fn op_resolve( state: &mut OpState, args: ResolveArgs, -) -> Result { +) -> Result, AnyError> { let state = state.borrow_mut::(); let mut resolved: Vec<(String, String)> = Vec::new(); let referrer = if let Some(remapped_specifier) = @@ -607,7 +607,7 @@ fn op_resolve( } } - Ok(json!(resolved)) + Ok(resolved) } #[derive(Debug, Deserialize, Eq, PartialEq)] @@ -917,9 +917,8 @@ mod tests { file_name: "cache:///some/file.js".to_string(), maybe_specifiers: Some(vec!["file:///some/file.ts".to_string()]), }, - ) - .expect("should have invoked op"); - assert_eq!(actual, json!(true)); + ); + assert!(actual); let state = state.borrow::(); assert_eq!(state.emitted_files.len(), 1); assert!(state.maybe_tsbuildinfo.is_none()); @@ -948,9 +947,8 @@ mod tests { vec!["file:///some/file.ts?q=.json".to_string()], ), }, - ) - .expect("should have invoked op"); - assert_eq!(actual, json!(true)); + ); + assert!(actual); let state = state.borrow::(); assert_eq!(state.emitted_files.len(), 1); assert!(state.maybe_tsbuildinfo.is_none()); @@ -977,9 +975,8 @@ mod tests { file_name: "deno:///.tsbuildinfo".to_string(), maybe_specifiers: None, }, - ) - .expect("should have invoked op"); - assert_eq!(actual, json!(true)); + ); + assert!(actual); let state = state.borrow::(); assert_eq!(state.emitted_files.len(), 0); assert_eq!( @@ -1095,7 +1092,10 @@ mod tests { }, ) .expect("should have invoked op"); - assert_eq!(actual, json!([["https://deno.land/x/b.ts", ".ts"]])); + assert_eq!( + actual, + vec![("https://deno.land/x/b.ts".into(), ".ts".into())] + ); } #[tokio::test] @@ -1116,7 +1116,7 @@ mod tests { .expect("should have not errored"); assert_eq!( actual, - json!([["deno:///missing_dependency.d.ts", ".d.ts"]]) + vec![("deno:///missing_dependency.d.ts".into(), ".d.ts".into())] ); } diff --git a/core/modules.rs b/core/modules.rs index aa7af64e3f..8c4f25d489 100644 --- a/core/modules.rs +++ b/core/modules.rs @@ -1141,7 +1141,6 @@ impl ModuleMap { #[cfg(test)] mod tests { use super::*; - use crate::error::AnyError; use crate::Extension; use crate::JsRuntime; use crate::RuntimeOptions; @@ -1474,10 +1473,10 @@ import "/a.js"; static DISPATCH_COUNT: AtomicUsize = AtomicUsize::new(0); #[op] - fn op_test(_: &mut OpState, control: u8) -> Result { + fn op_test(control: u8) -> u8 { DISPATCH_COUNT.fetch_add(1, Ordering::Relaxed); assert_eq!(control, 42); - Ok(43) + 43 } let ext = Extension::builder().ops(vec![op_test::decl()]).build(); diff --git a/ext/url/lib.rs b/ext/url/lib.rs index 394fd5902e..be229cec55 100644 --- a/ext/url/lib.rs +++ b/ext/url/lib.rs @@ -160,13 +160,11 @@ pub fn op_url_parse_search_params( } #[op] -pub fn op_url_stringify_search_params( - args: Vec<(String, String)>, -) -> Result { +pub fn op_url_stringify_search_params(args: Vec<(String, String)>) -> String { let search = form_urlencoded::Serializer::new(String::new()) .extend_pairs(args) .finish(); - Ok(search) + search } pub fn get_declaration() -> PathBuf { diff --git a/ext/web/blob.rs b/ext/web/blob.rs index 43121ee3a4..7da42e178c 100644 --- a/ext/web/blob.rs +++ b/ext/web/blob.rs @@ -163,11 +163,10 @@ impl BlobPart for SlicedBlobPart { pub fn op_blob_create_part( state: &mut deno_core::OpState, data: ZeroCopyBuf, -) -> Result { +) -> Uuid { let blob_store = state.borrow::(); let part = InMemoryBlobPart(data.to_vec()); - let id = blob_store.insert_part(Arc::new(part)); - Ok(id) + blob_store.insert_part(Arc::new(part)) } #[derive(Deserialize)] @@ -219,13 +218,9 @@ pub async fn op_blob_read_part( } #[op] -pub fn op_blob_remove_part( - state: &mut deno_core::OpState, - id: Uuid, -) -> Result<(), AnyError> { +pub fn op_blob_remove_part(state: &mut deno_core::OpState, id: Uuid) { let blob_store = state.borrow::(); blob_store.remove_part(&id); - Ok(()) } #[op] diff --git a/ext/web/compression.rs b/ext/web/compression.rs index f5ee38257f..9f5fcb8139 100644 --- a/ext/web/compression.rs +++ b/ext/web/compression.rs @@ -38,7 +38,7 @@ pub fn op_compression_new( state: &mut OpState, format: String, is_decoder: bool, -) -> Result { +) -> ResourceId { let w = Vec::new(); let inner = match (format.as_str(), is_decoder) { ("deflate", true) => Inner::DeflateDecoder(ZlibDecoder::new(w)), @@ -52,7 +52,7 @@ pub fn op_compression_new( _ => unreachable!(), }; let resource = CompressionResource(RefCell::new(inner)); - Ok(state.resource_table.add(resource)) + state.resource_table.add(resource) } #[op] diff --git a/ext/web/lib.rs b/ext/web/lib.rs index d14a4a5d5a..0d57bf10e7 100644 --- a/ext/web/lib.rs +++ b/ext/web/lib.rs @@ -180,13 +180,13 @@ fn b64_decode(input: &[u8]) -> Result, AnyError> { } #[op] -fn op_base64_encode(s: ZeroCopyBuf) -> Result { - Ok(b64_encode(&s)) +fn op_base64_encode(s: ZeroCopyBuf) -> String { + b64_encode(&s) } #[op] -fn op_base64_btoa(s: ByteString) -> Result { - Ok(b64_encode(s)) +fn op_base64_btoa(s: ByteString) -> String { + b64_encode(s) } fn b64_encode(s: impl AsRef<[u8]>) -> String { @@ -323,7 +323,7 @@ struct EncodeIntoResult { fn op_encoding_encode_into( input: String, mut buffer: ZeroCopyBuf, -) -> Result { +) -> EncodeIntoResult { // Since `input` is already UTF-8, we can simply find the last UTF-8 code // point boundary from input that fits in `buffer`, and copy the bytes up to // that point. @@ -347,18 +347,17 @@ fn op_encoding_encode_into( buffer[..boundary].copy_from_slice(input[..boundary].as_bytes()); - Ok(EncodeIntoResult { + EncodeIntoResult { // The `read` output parameter is measured in UTF-16 code units. read: input[..boundary].encode_utf16().count(), written: boundary, - }) + } } /// Creates a [`CancelHandle`] resource that can be used to cancel invocations of certain ops. #[op] -pub fn op_cancel_handle(state: &mut OpState) -> Result { - let rid = state.resource_table.add(CancelHandle::new()); - Ok(rid) +pub fn op_cancel_handle(state: &mut OpState) -> ResourceId { + state.resource_table.add(CancelHandle::new()) } pub fn get_declaration() -> PathBuf { diff --git a/ext/web/message_port.rs b/ext/web/message_port.rs index b193e6b9b2..06f7e3c522 100644 --- a/ext/web/message_port.rs +++ b/ext/web/message_port.rs @@ -109,7 +109,7 @@ impl Resource for MessagePortResource { #[op] pub fn op_message_port_create_entangled( state: &mut OpState, -) -> Result<(ResourceId, ResourceId), AnyError> { +) -> (ResourceId, ResourceId) { let (port1, port2) = create_entangled_message_port(); let port1_id = state.resource_table.add(MessagePortResource { @@ -122,7 +122,7 @@ pub fn op_message_port_create_entangled( cancel: CancelHandle::new(), }); - Ok((port1_id, port2_id)) + (port1_id, port2_id) } #[derive(Deserialize, Serialize)] diff --git a/ext/web/timers.rs b/ext/web/timers.rs index 670df35826..eee7707199 100644 --- a/ext/web/timers.rs +++ b/ext/web/timers.rs @@ -28,7 +28,7 @@ pub type StartTime = Instant; // If the High precision flag is not set, the // nanoseconds are rounded on 2ms. #[op] -pub fn op_now(state: &mut OpState, _argument: ()) -> Result +pub fn op_now(state: &mut OpState, _argument: ()) -> f64 where TP: TimersPermission + 'static, { @@ -44,9 +44,7 @@ where subsec_nanos -= subsec_nanos % reduced_time_precision; } - let result = (seconds * 1_000) as f64 + (subsec_nanos / 1_000_000.0); - - Ok(result) + (seconds * 1_000) as f64 + (subsec_nanos / 1_000_000.0) } pub struct TimerHandle(Rc); @@ -64,11 +62,10 @@ impl Resource for TimerHandle { /// Creates a [`TimerHandle`] resource that can be used to cancel invocations of /// [`op_sleep`]. #[op] -pub fn op_timer_handle(state: &mut OpState) -> Result { - let rid = state +pub fn op_timer_handle(state: &mut OpState) -> ResourceId { + state .resource_table - .add(TimerHandle(CancelHandle::new_rc())); - Ok(rid) + .add(TimerHandle(CancelHandle::new_rc())) } /// Waits asynchronously until either `millis` milliseconds have passed or the @@ -87,14 +84,10 @@ pub async fn op_sleep( } #[op] -pub fn op_sleep_sync( - state: &mut OpState, - millis: u64, -) -> Result<(), AnyError> +pub fn op_sleep_sync(state: &mut OpState, millis: u64) where TP: TimersPermission + 'static, { state.borrow::().check_unstable(state, "Deno.sleepSync"); std::thread::sleep(Duration::from_millis(millis)); - Ok(()) } diff --git a/runtime/ops/os.rs b/runtime/ops/os.rs index ad2ff60ea3..c74a423ab5 100644 --- a/runtime/ops/os.rs +++ b/runtime/ops/os.rs @@ -103,13 +103,12 @@ fn op_delete_env(state: &mut OpState, key: String) -> Result<(), AnyError> { } #[op] -fn op_set_exit_code(state: &mut OpState, code: i32) -> Result<(), AnyError> { +fn op_set_exit_code(state: &mut OpState, code: i32) { state.borrow_mut::>().store(code, Relaxed); - Ok(()) } #[op] -fn op_exit(state: &mut OpState) -> Result<(), AnyError> { +fn op_exit(state: &mut OpState) { let code = state.borrow::>().load(Relaxed); std::process::exit(code) } diff --git a/runtime/ops/web_worker.rs b/runtime/ops/web_worker.rs index 4137217bdd..184ebfddba 100644 --- a/runtime/ops/web_worker.rs +++ b/runtime/ops/web_worker.rs @@ -55,16 +55,15 @@ async fn op_worker_recv_message( } #[op] -fn op_worker_close(state: &mut OpState) -> Result<(), AnyError> { +fn op_worker_close(state: &mut OpState) { // Notify parent that we're finished let mut handle = state.borrow_mut::().clone(); handle.terminate(); - Ok(()) } #[op] -fn op_worker_get_type(state: &mut OpState) -> Result { +fn op_worker_get_type(state: &mut OpState) -> WebWorkerType { let handle = state.borrow::().clone(); - Ok(handle.worker_type) + handle.worker_type } diff --git a/runtime/ops/worker_host.rs b/runtime/ops/worker_host.rs index bd1f1e3f57..f8831d4e9d 100644 --- a/runtime/ops/worker_host.rs +++ b/runtime/ops/worker_host.rs @@ -258,16 +258,12 @@ fn op_create_worker( } #[op] -fn op_host_terminate_worker( - state: &mut OpState, - id: WorkerId, -) -> Result<(), AnyError> { +fn op_host_terminate_worker(state: &mut OpState, id: WorkerId) { if let Some(worker_thread) = state.borrow_mut::().remove(&id) { worker_thread.terminate(); } else { debug!("tried to terminate non-existent worker {}", id); } - Ok(()) } enum WorkerChannel {