Add hidden abortOnClose option to iterators

abstract-iterator.js

@@ -18,13 +18,15 @@ const kClosed = Symbol('closed')
 const kCloseCallbacks = Symbol('closeCallbacks')
 const kKeyEncoding = Symbol('keyEncoding')
 const kValueEncoding = Symbol('valueEncoding')
+const kAbortOnClose = Symbol('abortOnClose')
 const kLegacy = Symbol('legacy')
 const kKeys = Symbol('keys')
 const kValues = Symbol('values')
 const kLimit = Symbol('limit')
 const kCount = Symbol('count')
 
 const emptyOptions = Object.freeze({})
+const noop = () => {}
 let warnedEnd = false
 
 // This class is an internal utility for common functionality between AbstractIterator,
@@ -55,6 +57,12 @@ class CommonIterator {
     this[kLimit] = Number.isInteger(options.limit) && options.limit >= 0 ? options.limit : Infinity
     this[kCount] = 0
 
+    // Undocumented option to abort pending work on close(). Used by the
+    // many-level module as a temporary solution to a blocked close().
+    // TODO (next major): consider making this the default behavior. Native
+    // implementations should have their own logic to safely close iterators.
+    this[kAbortOnClose] = !!options.abortOnClose
+
     this.db = db
     this.db.attachResource(this)
     this.nextTick = db.nextTick
@@ -219,6 +227,9 @@ class CommonIterator {
   [kFinishWork] () {
     const cb = this[kCallback]
 
+    // Callback will be null if work was aborted on close
+    if (this[kAbortOnClose] && cb === null) return noop
+
     this[kWorking] = false
     this[kCallback] = null
 
@@ -277,6 +288,13 @@ class CommonIterator {
 
       if (!this[kWorking]) {
         this._close(this[kHandleClose])
+      } else if (this[kAbortOnClose]) {
+        // Don't wait for work to finish. Subsequently ignore the result.
+        const cb = this[kFinishWork]()
+
+        cb(new ModuleError('Aborted on iterator close()', {
+          code: 'LEVEL_ITERATOR_NOT_OPEN'
+        }))
       }
     }
 

abstract-level.js

@@ -654,8 +654,8 @@ class AbstractLevel extends EventEmitter {
     options.values = options.values !== false
 
     // We need the original encoding options in AbstractIterator in order to decode data
-    Object.defineProperty(options, AbstractIterator.keyEncoding, { value: keyEncoding })
-    Object.defineProperty(options, AbstractIterator.valueEncoding, { value: valueEncoding })
+    options[AbstractIterator.keyEncoding] = keyEncoding
+    options[AbstractIterator.valueEncoding] = valueEncoding
 
     // Forward encoding options to private API
     options.keyEncoding = keyEncoding.format
@@ -684,8 +684,8 @@ class AbstractLevel extends EventEmitter {
     options = rangeOptions(options, keyEncoding)
 
     // We need the original encoding options in AbstractKeyIterator in order to decode data
-    Object.defineProperty(options, AbstractIterator.keyEncoding, { value: keyEncoding })
-    Object.defineProperty(options, AbstractIterator.valueEncoding, { value: valueEncoding })
+    options[AbstractIterator.keyEncoding] = keyEncoding
+    options[AbstractIterator.valueEncoding] = valueEncoding
 
     // Forward encoding options to private API
     options.keyEncoding = keyEncoding.format
@@ -713,8 +713,8 @@ class AbstractLevel extends EventEmitter {
     options = rangeOptions(options, keyEncoding)
 
     // We need the original encoding options in AbstractValueIterator in order to decode data
-    Object.defineProperty(options, AbstractIterator.keyEncoding, { value: keyEncoding })
-    Object.defineProperty(options, AbstractIterator.valueEncoding, { value: valueEncoding })
+    options[AbstractIterator.keyEncoding] = keyEncoding
+    options[AbstractIterator.valueEncoding] = valueEncoding
 
     // Forward encoding options to private API
     options.keyEncoding = keyEncoding.format

test/iterator-test.js

@@ -127,7 +127,7 @@ exports.sequence = function (test, testCommon) {
 
         // NOTE: adapted from leveldown
         test(`${mode}().${method}() after db.close() yields error (deferred: ${deferred})`, async function (t) {
-          t.plan(1)
+          t.plan(2)
 
           const db = testCommon.factory()
           if (!deferred) await db.open()
@@ -137,22 +137,25 @@ exports.sequence = function (test, testCommon) {
 
           const it = db[mode]()
 
-          // The first call should succeed, because it was scheduled before close()
+          // The first call *should* succeed, because it was scheduled before close(). However, success
+          // is not a must. Because nextv() and all() fallback to next*(), they're allowed to fail. An
+          // implementation can also choose to abort any pending call on close.
           let promise = it[method](...requiredArgs).then(() => {
-            // The second call should fail, because it was scheduled after close()
-            return it[method](...requiredArgs).catch(err => {
-              t.is(err.code, 'LEVEL_ITERATOR_NOT_OPEN')
-            })
+            t.pass('Optionally succeeded')
+          }).catch((err) => {
+            t.is(err.code, 'LEVEL_ITERATOR_NOT_OPEN')
           })
 
-          if (method !== 'next') {
-            // However, because nextv() and all() fallback to next*(), they're allowed to fail too (for now)
-            promise = promise.catch((err) => {
+          // The second call *must* fail, because it was scheduled after close()
+          promise = promise.then(() => {
+            return it[method](...requiredArgs).then(() => {
+              t.fail('Expected an error')
+            }).catch((err) => {
               t.is(err.code, 'LEVEL_ITERATOR_NOT_OPEN')
             })
-          }
+          })
 
-          await Promise.all([db.close(), promise])
+          return Promise.all([db.close(), promise])
         })
       }
     }