Breaking: use undefined instead of error for non-existing entries (#49)

Closes Level/community#76.

README.md

@@ -99,7 +99,6 @@
     - [`del` (deprecated)](#del-deprecated)
     - [`batch` (deprecated)](#batch-deprecated)
   - [Errors](#errors)
-    - [`LEVEL_NOT_FOUND`](#level_not_found)
     - [`LEVEL_DATABASE_NOT_OPEN`](#level_database_not_open)
     - [`LEVEL_DATABASE_NOT_CLOSED`](#level_database_not_closed)
     - [`LEVEL_ITERATOR_NOT_OPEN`](#level_iterator_not_open)
@@ -296,7 +295,7 @@ Get a value from the database by `key`. The optional `options` object may contai
 - `keyEncoding`: custom key encoding for this operation, used to encode the `key`.
 - `valueEncoding`: custom value encoding for this operation, used to decode the value.
 
-The `callback` function will be called with an error if the operation failed. If the key was not found, the error will have code [`LEVEL_NOT_FOUND`](#errors). If successful the first argument will be `null` and the second argument will be the value. If no callback is provided, a promise is returned.
+The `callback` function will be called with an error if the operation failed. If successful the first argument will be nullish and the second argument will be the value. If the `key` was not found then the value will be `undefined`. If no callback is provided, a promise is returned.
 
 If the database indicates support of snapshots via `db.supports.snapshots` then `db.get()` _should_ read from a snapshot of the database, created at the time `db.get()` was called. This means it should not see the data of simultaneous write operations. However, this is currently not verified by the [abstract test suite](#test-suite).
 
@@ -1138,10 +1137,6 @@ A database may also throw [`TypeError`](https://developer.mozilla.org/en-US/docs
 
 Error codes will be one of the following.
 
-#### `LEVEL_NOT_FOUND`
-
-When a key was not found.
-
 #### `LEVEL_DATABASE_NOT_OPEN`
 
 When an operation was made on a database while it was closing or closed. Or when a database failed to `open()` including when `close()` was called in the mean time, thus changing the eventual `status`. The error may have a `cause` property that explains a failure to open:
@@ -1296,7 +1291,6 @@ Let's implement a basic in-memory database:
 
 ```js
 const { AbstractLevel } = require('abstract-level')
-const ModuleError = require('module-error')
 
 class ExampleLevel extends AbstractLevel {
   // This in-memory example doesn't have a location argument
@@ -1323,14 +1317,8 @@ class ExampleLevel extends AbstractLevel {
   }
 
   _get (key, options, callback) {
+    // Is undefined if not found
     const value = this._entries.get(key)
-
-    if (value === undefined) {
-      return this.nextTick(callback, new ModuleError(`Key ${key} was not found`, {
-        code: 'LEVEL_NOT_FOUND'
-      }))
-    }
-
     this.nextTick(callback, null, value)
   }
 
@@ -1420,9 +1408,9 @@ The default `_close()` is a sensible noop and invokes `callback` on a next tick.
 
 ### `db._get(key, options, callback)`
 
-Get a value by `key`. The `options` object will always have the following properties: `keyEncoding` and `valueEncoding`. If the key does not exist, call the `callback` function with an error that has code [`LEVEL_NOT_FOUND`](#errors). Otherwise call `callback` with `null` as the first argument and the value as the second.
+Get a value by `key`. The `options` object will always have the following properties: `keyEncoding` and `valueEncoding`. If an error occurs, call the `callback` function with an error. Otherwise call `callback` with `null` as the first argument and the value as the second. If the `key` was not found then use `undefined` as value.
 
-The default `_get()` invokes `callback` on a next tick with a `LEVEL_NOT_FOUND` error. It must be overridden.
+The default `_get()` invokes `callback` on a next tick with an `undefined` value. It must be overridden.
 
 ### `db._getMany(keys, options, callback)`
 

abstract-level.js

@@ -368,16 +368,14 @@ class AbstractLevel extends EventEmitter {
 
     this._get(this.prefixKey(keyEncoding.encode(key), keyFormat, true), options, (err, value) => {
       if (err) {
-        // Normalize not found error for backwards compatibility with abstract-leveldown and level(up)
-        if (err.code === 'LEVEL_NOT_FOUND' || err.notFound || /NotFound/i.test(err)) {
-          if (!err.code) err.code = 'LEVEL_NOT_FOUND' // Preferred way going forward
-          if (!err.notFound) err.notFound = true // Same as level-errors
-          if (!err.status) err.status = 404 // Same as level-errors
-        }
-
         return callback(err)
       }
 
+      // Entry was not found
+      if (value === undefined) {
+        return callback()
+      }
+
       try {
         value = valueEncoding.decode(value)
       } catch (err) {
@@ -394,7 +392,7 @@ class AbstractLevel extends EventEmitter {
   }
 
   _get (key, options, callback) {
-    this.nextTick(callback, new Error('NotFound'))
+    this.nextTick(callback, null, undefined)
   }
 
   getMany (keys, options, callback) {

test/batch-test.js

@@ -1,7 +1,7 @@
 'use strict'
 
 const { Buffer } = require('buffer')
-const { verifyNotFoundError, assertAsync } = require('./util')
+const { assertAsync } = require('./util')
 const { illegalKeys, illegalValues } = require('./util')
 
 let db
@@ -206,9 +206,8 @@ exports.batch = function (test, testCommon) {
       })
 
       db.get('foobatch2', function (err, value) {
-        t.ok(err, 'entry not found')
-        t.ok(typeof value === 'undefined', 'value is undefined')
-        t.ok(verifyNotFoundError(err), 'NotFound error')
+        t.error(err, 'no error')
+        t.is(value, undefined, 'not found')
         done()
       })
 
@@ -264,7 +263,7 @@ exports.batch = function (test, testCommon) {
 
 exports.atomic = function (test, testCommon) {
   test('test batch() is atomic', function (t) {
-    t.plan(4)
+    t.plan(6)
 
     let async = false
 
@@ -276,11 +275,13 @@ exports.atomic = function (test, testCommon) {
       t.ok(err, 'should error')
       t.ok(async, 'callback is asynchronous')
 
-      db.get('foobah1', function (err) {
-        t.ok(err, 'should not be found')
+      db.get('foobah1', function (err, value) {
+        t.error(err, 'no error')
+        t.is(value, undefined, 'should not be found')
       })
-      db.get('foobah3', function (err) {
-        t.ok(err, 'should not be found')
+      db.get('foobah3', function (err, value) {
+        t.error(err, 'no error')
+        t.is(value, undefined, 'should not be found')
       })
     })
 

test/deferred-open-test.js

@@ -13,8 +13,9 @@ exports.all = function (test, testCommon) {
         t.is(db.status, 'open', 'status is ok')
 
         if (--pendingGets <= 0) {
-          db.get('k4', { valueEncoding: 'utf8' }, function (err) {
-            t.ok(err)
+          db.get('k4', { valueEncoding: 'utf8' }, function (err, v) {
+            t.ifError(err, 'no get() error')
+            t.is(v, undefined, 'not found')
             db.close(t.ifError.bind(t))
           })
         }
@@ -24,7 +25,7 @@ exports.all = function (test, testCommon) {
 
   // NOTE: copied from levelup
   test('deferred open(): put() and get() on new database', function (t) {
-    t.plan(15)
+    t.plan(16)
 
     // Open database without callback, opens in next tick
     const db = testCommon.factory()
@@ -47,7 +48,7 @@ exports.all = function (test, testCommon) {
 
   // NOTE: copied from levelup
   test('deferred open(): batch() on new database', function (t) {
-    t.plan(13)
+    t.plan(14)
 
     // Open database without callback, opens in next tick
     const db = testCommon.factory()
@@ -67,7 +68,7 @@ exports.all = function (test, testCommon) {
 
   // NOTE: copied from levelup
   test('deferred open(): chained batch() on new database', function (t) {
-    t.plan(13)
+    t.plan(14)
 
     // Open database without callback, opens in next tick
     const db = testCommon.factory()

test/del-test.js

@@ -1,6 +1,6 @@
 'use strict'
 
-const { verifyNotFoundError, illegalKeys, assertAsync } = require('./util')
+const { illegalKeys, assertAsync } = require('./util')
 
 let db
 
@@ -36,9 +36,8 @@ exports.del = function (test, testCommon) {
       db.del('foo', function (err) {
         t.error(err)
         db.get('foo', function (err, value) {
-          t.ok(err, 'entry properly deleted')
-          t.ok(typeof value === 'undefined', 'value is undefined')
-          t.ok(verifyNotFoundError(err), 'NotFound error')
+          t.error(err, 'no error')
+          t.is(value, undefined, 'not found')
           t.end()
         })
       })
@@ -51,9 +50,8 @@ exports.del = function (test, testCommon) {
       db.del('foo').then(function (err) {
         t.error(err)
         db.get('foo', function (err, value) {
-          t.ok(err, 'entry properly deleted')
-          t.ok(typeof value === 'undefined', 'value is undefined')
-          t.ok(verifyNotFoundError(err), 'NotFound error')
+          t.error(err, 'no error')
+          t.is(value, undefined, 'not found')
           t.end()
         })
       }).catch(t.fail.bind(t))

test/factory-test.js

@@ -41,7 +41,7 @@ module.exports = function (test, testCommon) {
         db1.put('key', 'value', function (err) {
           t.error(err, 'put key in db1')
           db2.get('key', function (err, value) {
-            t.ok(err, 'db2 should be empty')
+            t.error(err, 'no error')
             t.is(value, undefined, 'db2 should be empty')
             close()
           })

test/get-test.js

@@ -1,7 +1,7 @@
 'use strict'
 
 const isBuffer = require('is-buffer')
-const { verifyNotFoundError, illegalKeys, assertAsync } = require('./util')
+const { illegalKeys, assertAsync } = require('./util')
 
 let db
 
@@ -69,9 +69,8 @@ exports.get = function (test, testCommon) {
       db.get('promises').then(function (value) {
         t.is(value, 'yes', 'got value without options')
 
-        db.get('not found').catch(function (err) {
-          t.ok(err, 'should error')
-          t.ok(verifyNotFoundError(err), 'correct error')
+        db.get('not found').then(function (value) {
+          t.is(value, undefined, 'not found')
 
           if (!db.supports.encodings.buffer) {
             return t.end()
@@ -105,24 +104,22 @@ exports.get = function (test, testCommon) {
 
       for (let i = 0; i < 10; ++i) {
         db.get('not found', function (err, value) {
-          t.ok(err, 'should error')
-          t.ok(verifyNotFoundError(err), 'correct error')
-          t.ok(typeof value === 'undefined', 'value is undefined')
+          t.error(err, 'no error')
+          t.is(value, undefined, 'not found')
           done()
         })
       }
     })
   })
 
   test('test get() not found error is asynchronous', function (t) {
-    t.plan(4)
+    t.plan(3)
 
     let async = false
 
     db.get('not found', function (err, value) {
-      t.ok(err, 'should error')
-      t.ok(verifyNotFoundError(err), 'correct error')
-      t.ok(typeof value === 'undefined', 'value is undefined')
+      t.error(err, 'no error')
+      t.is(value, undefined, 'not found')
       t.ok(async, 'callback is asynchronous')
     })
 

test/put-get-del-test.js

@@ -1,6 +1,5 @@
 'use strict'
 
-const { verifyNotFoundError } = require('./util')
 const { Buffer } = require('buffer')
 
 let db
@@ -28,9 +27,8 @@ function makeTest (test, type, key, value, expectedResult) {
           let async = false
 
           db.get(key, function (err, value) {
-            t.ok(err, 'entry properly deleted')
-            t.ok(verifyNotFoundError(err), 'correct error')
-            t.is(value, undefined, 'value is undefined')
+            t.error(err, 'no error')
+            t.is(value, undefined, 'not found')
             t.ok(async, 'callback is asynchronous')
             t.end()
           })

test/util.js

@@ -1,15 +1,10 @@
 'use strict'
 
-const ModuleError = require('module-error')
 const { AbstractLevel, AbstractChainedBatch } = require('..')
 const { AbstractIterator, AbstractKeyIterator, AbstractValueIterator } = require('..')
 
 const spies = []
 
-exports.verifyNotFoundError = function (err) {
-  return err.code === 'LEVEL_NOT_FOUND' && err.notFound === true && err.status === 404
-}
-
 exports.illegalKeys = [
   { name: 'null key', key: null },
   { name: 'undefined key', key: undefined }
@@ -130,14 +125,8 @@ class MinimalLevel extends AbstractLevel {
   }
 
   _get (key, options, callback) {
+    // Is undefined if not found
     const value = this[kEntries].get(key)
-
-    if (value === undefined) {
-      return this.nextTick(callback, new ModuleError(`Key ${key} was not found`, {
-        code: 'LEVEL_NOT_FOUND'
-      }))
-    }
-
     this.nextTick(callback, null, value)
   }