Add opt-in test for signal option (#59)

And require error `name` to be `AbortError`, as follow-up for
#55 (comment).

Author: vweevers

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()`
 

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'
   }

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"

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()
+        })
+      }
     }
   }
 }