Skip to content

Commit

Permalink
Improve error handling
Browse files Browse the repository at this point in the history
idb-keyval reported IDBTransaction.error if a `set` operation failed.  However, based on my reading of the IndexedDB spec, and based on testing, IDBTransaction.error may not be set at the time the `error` event fires. This can be seen under the following scenarios:

- If IDBObjectStore.add fails because a key already exists
- If a quota is exceeded in Safari
- If the transaction is aborted immediately after an IndexedDB operation is requested

The `error` event's EventTarget should give the specific IDBRequest that failed, and that IDBRequest's error property should always be populated, so accessing `event.target` should fix this issue.

I tried using a similar approach for `abort` events, but it did not work; strangely, the abort event's target appears to be the IDBOpenRequest associated with the transaction, rather than the transaction itself, so it does not have a usable error property.

This PR also adds limited unit tests to cover this scenario. Aborting a transaction immediately after issuing a database request appears to be a good way to force an error.

Fixes jakearchibald#163
  • Loading branch information
joshkel committed Jun 23, 2023
1 parent 6695b17 commit 8e72e26
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 2 deletions.
4 changes: 3 additions & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ export function promisifyRequest<T = undefined>(
// @ts-ignore - file size hacks
request.oncomplete = request.onsuccess = () => resolve(request.result);
// @ts-ignore - file size hacks
request.onabort = request.onerror = () => reject(request.error);
request.onerror = (e) => reject((e.target as IDBRequest<T> | IDBTransaction).error);
// @ts-ignore - file size hacks
request.onabort = () => reject(request.error);
});
}

Expand Down
30 changes: 29 additions & 1 deletion test/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ import {
setMany,
update,
getMany,
delMany
delMany,
UseStore
} from '../src';
import { assert as typeAssert, IsExact } from 'conditional-type-checks';

Expand All @@ -23,6 +24,11 @@ mocha.setup('tdd');
(async () => {
await promisifyRequest(indexedDB.deleteDatabase('keyval-store'));
const customStore = createStore('custom-db', 'custom-kv');
const errorStore: UseStore = (txMode, callback) => customStore(txMode, (store) => {
const result = callback(store);
store.transaction.abort();
return result;
})

suite('The basics', () => {
test('get & set', async () => {
Expand Down Expand Up @@ -152,6 +158,28 @@ mocha.setup('tdd');
'Error is correct type',
);
}

try {
await get('foo', errorStore);
assert.fail('Expected throw');
} catch (err) {
assert.strictEqual(
(err as DOMException).name,
'AbortError',
'Error is correct type',
);
}

try {
await set('foo', 'bar', errorStore);
assert.fail('Expected throw');
} catch (err) {
assert.strictEqual(
(err as DOMException).name,
'AbortError',
'Error is correct type',
);
}
});
});

Expand Down

0 comments on commit 8e72e26

Please sign in to comment.