Skip to content

Commit

Permalink
Add hidden abortOnClose option to iterators
Browse files Browse the repository at this point in the history
Will be used by `many-level`.
  • Loading branch information
vweevers committed Mar 20, 2022
1 parent eb08363 commit 2935180
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 11 deletions.
18 changes: 18 additions & 0 deletions abstract-iterator.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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'
}))
}
}

Expand Down
25 changes: 14 additions & 11 deletions test/iterator-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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])
})
}
}
Expand Down

0 comments on commit 2935180

Please sign in to comment.