From cb385d9e4acbd81235c3784d7e56b49c3fa41dd3 Mon Sep 17 00:00:00 2001 From: Leo Kettmeir Date: Mon, 14 Oct 2024 13:53:17 -0700 Subject: [PATCH] refactor(ext/webstorage): use concrete error types (#26173) --- Cargo.lock | 1 + ext/webstorage/Cargo.toml | 1 + ext/webstorage/lib.rs | 77 +++++++++++++------------------------- runtime/errors.rs | 15 +++++++- runtime/ops/worker_host.rs | 11 +++--- 5 files changed, 47 insertions(+), 58 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9625365e6b..a8e8608ddf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2235,6 +2235,7 @@ dependencies = [ "deno_core", "deno_web", "rusqlite", + "thiserror", ] [[package]] diff --git a/ext/webstorage/Cargo.toml b/ext/webstorage/Cargo.toml index 08ed8b0f29..bf9bbf23be 100644 --- a/ext/webstorage/Cargo.toml +++ b/ext/webstorage/Cargo.toml @@ -17,3 +17,4 @@ path = "lib.rs" deno_core.workspace = true deno_web.workspace = true rusqlite.workspace = true +thiserror.workspace = true diff --git a/ext/webstorage/lib.rs b/ext/webstorage/lib.rs index 99e61a1804..40946f05a7 100644 --- a/ext/webstorage/lib.rs +++ b/ext/webstorage/lib.rs @@ -2,10 +2,8 @@ // NOTE to all: use **cached** prepared statements when interfacing with SQLite. -use std::fmt; use std::path::PathBuf; -use deno_core::error::AnyError; use deno_core::op2; use deno_core::OpState; use rusqlite::params; @@ -14,6 +12,18 @@ use rusqlite::OptionalExtension; pub use rusqlite; +#[derive(Debug, thiserror::Error)] +pub enum WebStorageError { + #[error("LocalStorage is not supported in this context.")] + ContextNotSupported, + #[error(transparent)] + Sqlite(#[from] rusqlite::Error), + #[error(transparent)] + Io(std::io::Error), + #[error("Exceeded maximum storage size")] + StorageExceeded, +} + #[derive(Clone)] struct OriginStorageDir(PathBuf); @@ -51,15 +61,13 @@ struct SessionStorage(Connection); fn get_webstorage( state: &mut OpState, persistent: bool, -) -> Result<&Connection, AnyError> { +) -> Result<&Connection, WebStorageError> { let conn = if persistent { if state.try_borrow::().is_none() { - let path = state.try_borrow::().ok_or_else(|| { - DomExceptionNotSupportedError::new( - "LocalStorage is not supported in this context.", - ) - })?; - std::fs::create_dir_all(&path.0)?; + let path = state + .try_borrow::() + .ok_or(WebStorageError::ContextNotSupported)?; + std::fs::create_dir_all(&path.0).map_err(WebStorageError::Io)?; let conn = Connection::open(path.0.join("local_storage"))?; // Enable write-ahead-logging and tweak some other stuff. let initial_pragmas = " @@ -106,7 +114,7 @@ fn get_webstorage( pub fn op_webstorage_length( state: &mut OpState, persistent: bool, -) -> Result { +) -> Result { let conn = get_webstorage(state, persistent)?; let mut stmt = conn.prepare_cached("SELECT COUNT(*) FROM data")?; @@ -121,7 +129,7 @@ pub fn op_webstorage_key( state: &mut OpState, #[smi] index: u32, persistent: bool, -) -> Result, AnyError> { +) -> Result, WebStorageError> { let conn = get_webstorage(state, persistent)?; let mut stmt = @@ -135,14 +143,9 @@ pub fn op_webstorage_key( } #[inline] -fn size_check(input: usize) -> Result<(), AnyError> { +fn size_check(input: usize) -> Result<(), WebStorageError> { if input >= MAX_STORAGE_BYTES { - return Err( - deno_web::DomExceptionQuotaExceededError::new( - "Exceeded maximum storage size", - ) - .into(), - ); + return Err(WebStorageError::StorageExceeded); } Ok(()) @@ -154,7 +157,7 @@ pub fn op_webstorage_set( #[string] key: &str, #[string] value: &str, persistent: bool, -) -> Result<(), AnyError> { +) -> Result<(), WebStorageError> { let conn = get_webstorage(state, persistent)?; size_check(key.len() + value.len())?; @@ -178,7 +181,7 @@ pub fn op_webstorage_get( state: &mut OpState, #[string] key_name: String, persistent: bool, -) -> Result, AnyError> { +) -> Result, WebStorageError> { let conn = get_webstorage(state, persistent)?; let mut stmt = conn.prepare_cached("SELECT value FROM data WHERE key = ?")?; @@ -194,7 +197,7 @@ pub fn op_webstorage_remove( state: &mut OpState, #[string] key_name: &str, persistent: bool, -) -> Result<(), AnyError> { +) -> Result<(), WebStorageError> { let conn = get_webstorage(state, persistent)?; let mut stmt = conn.prepare_cached("DELETE FROM data WHERE key = ?")?; @@ -207,7 +210,7 @@ pub fn op_webstorage_remove( pub fn op_webstorage_clear( state: &mut OpState, persistent: bool, -) -> Result<(), AnyError> { +) -> Result<(), WebStorageError> { let conn = get_webstorage(state, persistent)?; let mut stmt = conn.prepare_cached("DELETE FROM data")?; @@ -221,7 +224,7 @@ pub fn op_webstorage_clear( pub fn op_webstorage_iterate_keys( state: &mut OpState, persistent: bool, -) -> Result, AnyError> { +) -> Result, WebStorageError> { let conn = get_webstorage(state, persistent)?; let mut stmt = conn.prepare_cached("SELECT key FROM data")?; @@ -232,31 +235,3 @@ pub fn op_webstorage_iterate_keys( Ok(keys) } - -#[derive(Debug)] -pub struct DomExceptionNotSupportedError { - pub msg: String, -} - -impl DomExceptionNotSupportedError { - pub fn new(msg: &str) -> Self { - DomExceptionNotSupportedError { - msg: msg.to_string(), - } - } -} - -impl fmt::Display for DomExceptionNotSupportedError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.pad(&self.msg) - } -} - -impl std::error::Error for DomExceptionNotSupportedError {} - -pub fn get_not_supported_error_class_name( - e: &AnyError, -) -> Option<&'static str> { - e.downcast_ref::() - .map(|_| "DOMExceptionNotSupportedError") -} diff --git a/runtime/errors.rs b/runtime/errors.rs index 4c6aeab98a..bc28339ad7 100644 --- a/runtime/errors.rs +++ b/runtime/errors.rs @@ -18,6 +18,7 @@ use deno_core::url; use deno_core::ModuleResolutionError; use deno_cron::CronError; use deno_tls::TlsError; +use deno_webstorage::WebStorageError; use std::env; use std::error::Error; use std::io; @@ -158,6 +159,15 @@ pub fn get_nix_error_class(error: &nix::Error) -> &'static str { } } +fn get_webstorage_class_name(e: &WebStorageError) -> &'static str { + match e { + WebStorageError::ContextNotSupported => "DOMExceptionNotSupportedError", + WebStorageError::Sqlite(_) => todo!(), + WebStorageError::Io(e) => get_io_error_class(e), + WebStorageError::StorageExceeded => "DOMExceptionQuotaExceededError", + } +} + fn get_tls_error_class(e: &TlsError) -> &'static str { match e { TlsError::Rustls(_) => "Error", @@ -221,7 +231,6 @@ pub fn get_error_class_name(e: &AnyError) -> Option<&'static str> { deno_core::error::get_custom_error_class(e) .or_else(|| deno_webgpu::error::get_error_class_name(e)) .or_else(|| deno_web::get_error_class_name(e)) - .or_else(|| deno_webstorage::get_not_supported_error_class_name(e)) .or_else(|| deno_websocket::get_network_error_class_name(e)) .or_else(|| e.downcast_ref::().map(get_tls_error_class)) .or_else(|| e.downcast_ref::().map(get_cron_error_class)) @@ -231,6 +240,10 @@ pub fn get_error_class_name(e: &AnyError) -> Option<&'static str> { e.downcast_ref::() .map(get_broadcast_channel_error) }) + .or_else(|| { + e.downcast_ref::() + .map(get_webstorage_class_name) + }) .or_else(|| { e.downcast_ref::() .map(get_dlopen_error_class) diff --git a/runtime/ops/worker_host.rs b/runtime/ops/worker_host.rs index b9fd066543..61e5ef3e0a 100644 --- a/runtime/ops/worker_host.rs +++ b/runtime/ops/worker_host.rs @@ -10,6 +10,7 @@ use crate::web_worker::WorkerControlEvent; use crate::web_worker::WorkerId; use crate::web_worker::WorkerMetadata; use crate::worker::FormatJsErrorFn; +use deno_core::error::custom_error; use deno_core::error::AnyError; use deno_core::op2; use deno_core::serde::Deserialize; @@ -136,12 +137,10 @@ fn op_create_worker( let worker_type = args.worker_type; if let WebWorkerType::Classic = worker_type { if let TestingFeaturesEnabled(false) = state.borrow() { - return Err( - deno_webstorage::DomExceptionNotSupportedError::new( - "Classic workers are not supported.", - ) - .into(), - ); + return Err(custom_error( + "DOMExceptionNotSupportedError", + "Classic workers are not supported.", + )); } }