[IndexedDB] Reduce heed related panics (#37652)

Allows indexeddb backends to return errors on certain operations.
Currently the errors are not demarcated, as the result type is
`Result<(), ()>`. If this is not appropriate then perhaps having a
string error might be better.

Testing: Some tests might perhaps move from PANIC to FAIL
Fixes: Partially fixes a bit of #37647, more work needs to be done
however

---------

Signed-off-by: Ashwin Naren <arihant2math@gmail.com>
Signed-off-by: Josh Matthews <josh@joshmatthews.net>
Co-authored-by: Josh Matthews <josh@joshmatthews.net>
This commit is contained in:
Ashwin Naren 2025-06-24 17:22:19 -07:00 committed by GitHub
parent 72e0baa997
commit d114feb0fa
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 60 additions and 28 deletions

View file

@ -61,16 +61,14 @@ impl HeedEngine {
} }
impl KvsEngine for HeedEngine { impl KvsEngine for HeedEngine {
fn create_store(&self, store_name: SanitizedName, auto_increment: bool) { type Error = heed::Error;
let mut write_txn = self
.heed_env fn create_store(&self, store_name: SanitizedName, auto_increment: bool) -> heed::Result<()> {
.write_txn() let mut write_txn = self.heed_env.write_txn()?;
.expect("Could not create idb store writer");
let _ = self.heed_env.clear_stale_readers(); let _ = self.heed_env.clear_stale_readers();
let new_store: HeedDatabase = self let new_store: HeedDatabase = self
.heed_env .heed_env
.create_database(&mut write_txn, Some(&*store_name.to_string())) .create_database(&mut write_txn, Some(&*store_name.to_string()))?;
.expect("Failed to create idb store");
write_txn.commit().expect("Failed to commit transaction"); write_txn.commit().expect("Failed to commit transaction");
@ -85,30 +83,29 @@ impl KvsEngine for HeedEngine {
.write() .write()
.expect("Could not acquire lock on stores") .expect("Could not acquire lock on stores")
.insert(store_name, store); .insert(store_name, store);
Ok(())
} }
fn delete_store(&self, store_name: SanitizedName) { fn delete_store(&self, store_name: SanitizedName) -> heed::Result<()> {
// TODO: Actually delete store instead of just clearing it // TODO: Actually delete store instead of just clearing it
let mut write_txn = self let mut write_txn = self.heed_env.write_txn()?;
.heed_env
.write_txn()
.expect("Could not create idb store writer");
let store: HeedDatabase = self let store: HeedDatabase = self
.heed_env .heed_env
.create_database(&mut write_txn, Some(&*store_name.to_string())) .create_database(&mut write_txn, Some(&*store_name.to_string()))?;
.expect("Failed to create idb store"); store.clear(&mut write_txn)?;
store.clear(&mut write_txn).expect("Could not clear store");
write_txn.commit().expect("Failed to commit transaction"); write_txn.commit().expect("Failed to commit transaction");
let mut open_stores = self.open_stores.write().unwrap(); let mut open_stores = self.open_stores.write().unwrap();
open_stores.retain(|key, _| key != &store_name); open_stores.retain(|key, _| key != &store_name);
Ok(())
} }
fn close_store(&self, store_name: SanitizedName) { fn close_store(&self, store_name: SanitizedName) -> heed::Result<()> {
// FIXME: (arihant2math) unused // FIXME: (arihant2math) unused
// FIXME:(arihant2math) return error if no store ... // FIXME:(arihant2math) return error if no store ...
let mut open_stores = self.open_stores.write().unwrap(); let mut open_stores = self.open_stores.write().unwrap();
open_stores.retain(|key, _| key != &store_name); open_stores.retain(|key, _| key != &store_name);
Ok(())
} }
// Starts a transaction, processes all operations for that transaction, // Starts a transaction, processes all operations for that transaction,
@ -162,12 +159,14 @@ impl KvsEngine for HeedEngine {
if tx.send(None).is_err() { if tx.send(None).is_err() {
warn!("IDBTransaction's execution channel is dropped"); warn!("IDBTransaction's execution channel is dropped");
}; };
rtxn.commit().expect("Failed to commit transaction");
}); });
} else { } else {
self.write_pool.spawn(move || { self.write_pool.spawn(move || {
// Acquiring a writer will block the thread if another `readwrite` transaction is active // Acquiring a writer will block the thread if another `readwrite` transaction is active
let env = heed_env; let env = heed_env;
let mut wtxn = env.write_txn().expect("Could not creat idb store writer"); let mut wtxn = env.write_txn().expect("Could not create idb store writer");
for request in transaction.requests { for request in transaction.requests {
match request.operation { match request.operation {
AsyncOperation::PutItem(key, value, overwrite) => { AsyncOperation::PutItem(key, value, overwrite) => {

View file

@ -57,12 +57,18 @@ pub struct KvsTransaction {
} }
pub trait KvsEngine { pub trait KvsEngine {
fn create_store(&self, store_name: SanitizedName, auto_increment: bool); type Error;
fn delete_store(&self, store_name: SanitizedName); fn create_store(
&self,
store_name: SanitizedName,
auto_increment: bool,
) -> Result<(), Self::Error>;
fn delete_store(&self, store_name: SanitizedName) -> Result<(), Self::Error>;
#[expect(dead_code)] #[expect(dead_code)]
fn close_store(&self, store_name: SanitizedName); fn close_store(&self, store_name: SanitizedName) -> Result<(), Self::Error>;
fn process_transaction( fn process_transaction(
&self, &self,

View file

@ -135,9 +135,13 @@ impl<E: KvsEngine> IndexedDBEnvironment<E> {
store_name: SanitizedName, store_name: SanitizedName,
auto_increment: bool, auto_increment: bool,
) { ) {
self.engine.create_store(store_name, auto_increment); let result = self.engine.create_store(store_name, auto_increment);
let _ = sender.send(Ok(())); if result.is_ok() {
let _ = sender.send(Ok(()));
} else {
let _ = sender.send(Err(()));
}
} }
fn delete_object_store( fn delete_object_store(
@ -145,9 +149,13 @@ impl<E: KvsEngine> IndexedDBEnvironment<E> {
sender: IpcSender<Result<(), ()>>, sender: IpcSender<Result<(), ()>>,
store_name: SanitizedName, store_name: SanitizedName,
) { ) {
self.engine.delete_store(store_name); let result = self.engine.delete_store(store_name);
let _ = sender.send(Ok(())); if result.is_ok() {
let _ = sender.send(Ok(()));
} else {
let _ = sender.send(Err(()));
}
} }
} }

View file

@ -1,8 +1,24 @@
[list_ordering.any.worker.html] [list_ordering.any.worker.html]
expected: CRASH [Validate ObjectStoreNames and indexNames list order - numbers]
expected: FAIL
[Validate ObjectStoreNames and indexNames list order - numbers 'overflow']
expected: FAIL
[Validate ObjectStoreNames and indexNames list order - lexigraphical string sort]
expected: FAIL
[list_ordering.any.html] [list_ordering.any.html]
expected: CRASH [Validate ObjectStoreNames and indexNames list order - numbers]
expected: FAIL
[Validate ObjectStoreNames and indexNames list order - numbers 'overflow']
expected: FAIL
[Validate ObjectStoreNames and indexNames list order - lexigraphical string sort]
expected: FAIL
[list_ordering.any.sharedworker.html] [list_ordering.any.sharedworker.html]
expected: ERROR expected: ERROR

View file

@ -2,10 +2,13 @@
expected: ERROR expected: ERROR
[string-list-ordering.any.worker.html] [string-list-ordering.any.worker.html]
expected: CRASH [Test string list ordering in IndexedDB]
expected: FAIL
[string-list-ordering.any.serviceworker.html] [string-list-ordering.any.serviceworker.html]
expected: ERROR expected: ERROR
[string-list-ordering.any.html] [string-list-ordering.any.html]
expected: CRASH [Test string list ordering in IndexedDB]
expected: FAIL