diff --git a/README.md b/README.md index 0dd959a..ab7d39b 100644 --- a/README.md +++ b/README.md @@ -568,7 +568,7 @@ db.foo = async function (key) { The optional `options` object may contain: -- `signal`: an [`AbortSignal`](https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal) to abort the deferred operation. When aborted (now or later) the `fn` function will not be called, and the promise returned by `deferAsync()` will be rejected with a [`LEVEL_ABORTED`](#errors) error. +- `signal`: an [`AbortSignal`](https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal) to abort the deferred operation. When aborted (now or later) the `fn` function will not be called, and the promise returned by `deferAsync()` will be rejected with a [`LEVEL_ABORTED`](#level_aborted) error. ### `chainedBatch` @@ -734,7 +734,7 @@ const remaining = iterator.limit - iterator.count #### Aborting Iterators -Iterators take an experimental `signal` option that, once signaled, aborts an in-progress read operation (if any) and rejects subsequent reads. The relevant promise will be rejected with a [`LEVEL_ABORTED`](#errors) error. Aborting does not close the iterator, because closing is asynchronous and may result in an error that needs a place to go. This means signals should be used together with a pattern that automatically closes the iterator: +Iterators take an experimental `signal` option that, once signaled, aborts an in-progress read operation (if any) and rejects subsequent reads. The relevant promise will be rejected with a [`LEVEL_ABORTED`](#level_aborted) error. Aborting does not close the iterator, because closing is asynchronous and may result in an error that needs a place to go. This means signals should be used together with a pattern that automatically closes the iterator: ```js const abortController = new AbortController() @@ -770,6 +770,8 @@ try { } ``` +Support of signals is indicated via [`db.supports.signals.iterators`](https://github.com/Level/supports#signals-object). + ### `keyIterator` A key iterator has the same interface as `iterator` except that its methods yield keys instead of entries. Usage is otherwise the same. @@ -1212,7 +1214,13 @@ When an operation was made on a chained batch while it was closing or closed, wh #### `LEVEL_ABORTED` -When an operation was aborted by the user. +When an operation was aborted by the user. For [web compatibility](https://dom.spec.whatwg.org/#aborting-ongoing-activities) this error can also be identified by its `name` which is `'AbortError'`: + +```js +if (err.name === 'AbortError') { + // Operation was aborted +} +``` #### `LEVEL_ENCODING_NOT_FOUND` @@ -1617,13 +1625,13 @@ class ExampleSublevel extends AbstractSublevel { The first argument to this constructor must be an instance of the relevant `AbstractLevel` implementation. The constructor will set `iterator.db` which is used (among other things) to access encodings and ensures that `db` will not be garbage collected in case there are no other references to it. The `options` argument must be the original `options` object that was passed to `db._iterator()` and it is therefore not (publicly) possible to create an iterator via constructors alone. -The `signal` option, if any and once signaled, should abort an in-progress `_next()`, `_nextv()` or `_all()` call and reject the promise returned by that call with a [`LEVEL_ABORTED`](#errors) error. Doing so is optional until a future semver-major release. Responsibilities are divided as follows: +The `signal` option, if any and once signaled, should abort an in-progress `_next()`, `_nextv()` or `_all()` call and reject the promise returned by that call with a [`LEVEL_ABORTED`](#level_aborted) error. Doing so is optional until a future semver-major release. Responsibilities are divided as follows: 1. Before a database has finished opening, `abstract-level` handles the signal 2. While a call is in progress, the implementation handles the signal 3. Once the signal is aborted, `abstract-level` rejects further calls. -A method like `_next()` therefore doesn't have to check the signal _before_ it start its asynchronous work, only _during_ that work. Whether to respect the signal and on which (potentially long-running) methods, is up to the implementation. +A method like `_next()` therefore doesn't have to check the signal _before_ it start its asynchronous work, only _during_ that work. If supported, set `db.supports.signals.iterators` to `true` (via the manifest passed to the database constructor) which also enables relevant tests in the [test suite](#test-suite). #### `iterator._next()` diff --git a/lib/errors.js b/lib/errors.js index 3a88ed1..0960a89 100644 --- a/lib/errors.js +++ b/lib/errors.js @@ -10,13 +10,9 @@ class AbortError extends ModuleError { }) } - // TODO: we should set name to AbortError for web compatibility. See: + // Set name to AbortError for web compatibility. See: // https://dom.spec.whatwg.org/#aborting-ongoing-activities // https://github.com/nodejs/node/pull/35911#discussion_r515779306 - // - // But I'm not sure we can do the same for errors created by a Node-API addon (like - // classic-level) so for now this behavior is undocumented and folks should use the - // LEVEL_ABORTED code instead. get name () { return 'AbortError' } diff --git a/package.json b/package.json index ece90b4..e893a25 100644 --- a/package.json +++ b/package.json @@ -27,7 +27,7 @@ "dependencies": { "buffer": "^6.0.3", "is-buffer": "^2.0.5", - "level-supports": "^5.0.0", + "level-supports": "^6.0.0", "level-transcoder": "^1.0.1", "maybe-combine-errors": "^1.0.0", "module-error": "^1.0.1" diff --git a/test/iterator-test.js b/test/iterator-test.js index e78c4d1..22430b3 100644 --- a/test/iterator-test.js +++ b/test/iterator-test.js @@ -133,26 +133,76 @@ exports.sequence = function (test, testCommon) { }) } - // At the moment, we can only be sure that signals are supported if the iterator is deferred - globalThis.AbortController && test(`${mode}().${method}() with aborted signal yields error`, async function (t) { - t.plan(2) + // 1) At the moment, we can only be sure that signals are supported if the iterator is deferred + if (globalThis.AbortController) { + test(`${mode}().${method}() with aborted signal yields error (deferred)`, async function (t) { + t.plan(3) - const db = testCommon.factory() - const ac = new globalThis.AbortController() - const it = db[mode]({ signal: ac.signal }) + const db = testCommon.factory() + const ac = new globalThis.AbortController() + const it = db[mode]({ signal: ac.signal }) - t.is(db.status, 'opening', 'is deferred') - ac.abort() + t.is(db.status, 'opening', 'is deferred') + ac.abort() - try { + try { + await it[method](...requiredArgs) + } catch (err) { + t.is(err.code, 'LEVEL_ABORTED') + t.is(err.name, 'AbortError') + } + + await it.close() + return db.close() + }) + } + + // 2) Unless the implementation opts-in + if (globalThis.AbortController && testCommon.supports.signals && testCommon.supports.signals.iterators) { + test(`${mode}().${method}() with signal yields error when aborted`, async function (t) { + t.plan(2) + + const db = testCommon.factory() + + await db.open() + await db.batch().put('a', 'a').put('b', 'b').write() + + const ac = new globalThis.AbortController() + const it = db[mode]({ signal: ac.signal }) + const promise = it[method](...requiredArgs) + + ac.abort() + + try { + await promise + } catch (err) { + t.is(err.code, 'LEVEL_ABORTED') + t.is(err.name, 'AbortError') + } + + await it.close() + return db.close() + }) + + test(`${mode}().${method}() with non-aborted signal`, async function (t) { + const db = testCommon.factory() + + await db.open() + await db.batch().put('a', 'a').put('b', 'b').write() + + const ac = new globalThis.AbortController() + const it = db[mode]({ signal: ac.signal }) + + // We're merely testing that this does not throw. And implicitly testing (through + // coverage) that abort listeners are removed. An implementation might choose to + // periodically check signal.aborted instead of using an abort listener, so we + // can't directly assert that cleanup indeed happens. await it[method](...requiredArgs) - } catch (err) { - t.is(err.code, 'LEVEL_ABORTED') - } + await it.close() - await it.close() - return db.close() - }) + return db.close() + }) + } } } }