Skip to content

Commit

Permalink
chore: update err-code dependency (ipfs#2287)
Browse files Browse the repository at this point in the history
* chore: update err-code dependency

`err-code` no longer takes a string as the first argument.

This PR also removes a bunch of useless DEBUG logs that were logging just the error message and adds logging where a caught error is not directly passed back to callbacks. This should help with debuggability.

License: MIT
Signed-off-by: Alan Shaw <[email protected]>

* fix: name tests

License: MIT
Signed-off-by: Alan Shaw <[email protected]>

* refactor: republisher error msg

Co-Authored-By: dirkmc <[email protected]>

* refactor: republisher error msg

Co-Authored-By: dirkmc <[email protected]>
  • Loading branch information
Alan Shaw and dirkmc authored Jul 23, 2019
1 parent a5b590c commit bb4dc19
Show file tree
Hide file tree
Showing 11 changed files with 44 additions and 121 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@
"datastore-pubsub": "~0.1.1",
"debug": "^4.1.0",
"dlv": "^1.1.3",
"err-code": "^1.1.2",
"err-code": "^2.0.0",
"file-type": "^12.0.1",
"fnv1a": "^1.0.1",
"fsm-event": "^2.1.0",
Expand Down
8 changes: 2 additions & 6 deletions src/core/components/name-pubsub.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@ const isNamePubsubEnabled = (node) => {
// Get pubsub from IPNS routing
const getPubsubRouting = (node) => {
if (!node._ipns || !node._options.EXPERIMENTAL.ipnsPubsub) {
const errMsg = 'IPNS pubsub subsystem is not enabled'

throw errcode(errMsg, 'ERR_IPNS_PUBSUB_NOT_ENABLED')
throw errcode(new Error('IPNS pubsub subsystem is not enabled'), 'ERR_IPNS_PUBSUB_NOT_ENABLED')
}

// Only one store and it is pubsub
Expand All @@ -35,9 +33,7 @@ const getPubsubRouting = (node) => {
const pubsub = (node._ipns.routing.stores || []).find(s => IpnsPubsubDatastore.isIpnsPubsubDatastore(s))

if (!pubsub) {
const errMsg = 'IPNS pubsub datastore not found'

throw errcode(errMsg, 'ERR_PUBSUB_DATASTORE_NOT_FOUND')
throw errcode(new Error('IPNS pubsub datastore not found'), 'ERR_PUBSUB_DATASTORE_NOT_FOUND')
}

return pubsub
Expand Down
18 changes: 5 additions & 13 deletions src/core/components/name.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,7 @@ module.exports = function name (self) {
const key = options.key || 'self'

if (!self.isOnline()) {
const errMsg = utils.OFFLINE_ERROR

log.error(errMsg)
return callback(errcode(errMsg, 'OFFLINE_ERROR'))
return callback(errcode(new Error(utils.OFFLINE_ERROR), 'OFFLINE_ERROR'))
}

// TODO: params related logic should be in the core implementation
Expand Down Expand Up @@ -162,10 +159,7 @@ module.exports = function name (self) {

// TODO: params related logic should be in the core implementation
if (offline && options.nocache) {
const error = 'cannot specify both offline and nocache'

log.error(error)
return callback(errcode(new Error(error), 'ERR_NOCACHE_AND_OFFLINE'))
return callback(errcode(new Error('cannot specify both offline and nocache'), 'ERR_NOCACHE_AND_OFFLINE'))
}

// Set node id as name for being resolved, if it is not received
Expand All @@ -187,17 +181,15 @@ module.exports = function name (self) {
}

log.error(err)
return callback(errcode(new Error('Invalid IPNS name.'), 'ERR_IPNS_INVALID_NAME'))
return callback(errcode(new Error('Invalid IPNS name'), 'ERR_IPNS_INVALID_NAME'))
}

// multihash is valid lets resolve with IPNS
// IPNS resolve needs a online daemon
if (!self.isOnline() && !offline) {
const errMsg = utils.OFFLINE_ERROR

log.error(errMsg)
return callback(errcode(errMsg, 'OFFLINE_ERROR'))
return callback(errcode(new Error(utils.OFFLINE_ERROR), 'OFFLINE_ERROR'))
}

self._ipns.resolve(`/${namespace}/${hash}`, options, appendRemainder(callback, remainder))
}),
pubsub: namePubsub(self)
Expand Down
5 changes: 1 addition & 4 deletions src/core/ipns/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,7 @@ class IPNS {
// Resolve
resolve (name, options, callback) {
if (typeof name !== 'string') {
const errMsg = `name received is not valid`

log.error(errMsg)
return callback(errcode(new Error(errMsg), 'ERR_INVALID_NAME'))
return callback(errcode(new Error('name received is not valid'), 'ERR_INVALID_NAME'))
}

if (typeof options === 'function') {
Expand Down
5 changes: 1 addition & 4 deletions src/core/ipns/publisher.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,7 @@ class IpnsPublisher {
// publish record with a eol
publishWithEOL (privKey, value, lifetime, callback) {
if (!privKey || !privKey.bytes) {
const errMsg = `one or more of the provided parameters are not defined`

log.error(errMsg)
return callback(errcode(new Error(errMsg), 'ERR_UNDEFINED_PARAMETER'))
return callback(errcode(new Error('invalid private key'), 'ERR_INVALID_PRIVATE_KEY'))
}

PeerId.createFromPrivKey(privKey.bytes, (err, peerId) => {
Expand Down
36 changes: 8 additions & 28 deletions src/core/ipns/republisher.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,7 @@ class IpnsRepublisher {

start () {
if (this._republishHandle) {
const errMsg = 'already running'

log.error(errMsg)
throw errcode(new Error(errMsg), 'ERR_REPUBLISH_ALREADY_RUNNING')
throw errcode(new Error('republisher is already running'), 'ERR_REPUBLISH_ALREADY_RUNNING')
}

// TODO: this handler should be isolated in another module
Expand Down Expand Up @@ -78,10 +75,7 @@ class IpnsRepublisher {
const republishHandle = this._republishHandle

if (!republishHandle) {
const errMsg = 'not running'

log.error(errMsg)
return callback(errcode(new Error(errMsg), 'ERR_REPUBLISH_NOT_RUNNING'))
return callback(errcode(new Error('republisher is not running'), 'ERR_REPUBLISH_NOT_RUNNING'))
}

this._republishHandle = null
Expand Down Expand Up @@ -134,10 +128,7 @@ class IpnsRepublisher {

_republishEntry (privateKey, callback) {
if (!privateKey || !privateKey.bytes) {
const errMsg = `one or more of the provided parameters are not defined`

log.error(errMsg)
return callback(errcode(new Error(errMsg), 'ERR_UNDEFINED_PARAMETER'))
return callback(errcode(new Error('invalid private key'), 'ERR_INVALID_PRIVATE_KEY'))
}

waterfall([
Expand All @@ -154,40 +145,29 @@ class IpnsRepublisher {

_getPreviousValue (peerId, callback) {
if (!(PeerId.isPeerId(peerId))) {
const errMsg = `peerId received is not valid`

log.error(errMsg)
return callback(errcode(new Error(errMsg), 'ERR_INVALID_PEER_ID'))
return callback(errcode(new Error('invalid peer ID'), 'ERR_INVALID_PEER_ID'))
}

this._datastore.get(ipns.getLocalKey(peerId.id), (err, dsVal) => {
// error handling
// no need to republish
if (err && err.notFound) {
const errMsg = `no previous entry for record with id: ${peerId.id}`

log.error(errMsg)
return callback(errcode(new Error(errMsg), 'ERR_NO_ENTRY_FOUND'))
return callback(errcode(new Error(`no previous entry for record with id: ${peerId.id}`), 'ERR_NO_ENTRY_FOUND'))
} else if (err) {
return callback(err)
}

if (!Buffer.isBuffer(dsVal)) {
const errMsg = `found ipns record that we couldn't process`

log.error(errMsg)
return callback(errcode(new Error(errMsg), 'ERR_INVALID_IPNS_RECORD'))
return callback(errcode(new Error("found ipns record that we couldn't process"), 'ERR_INVALID_IPNS_RECORD'))
}

// unmarshal data
let record
try {
record = ipns.unmarshal(dsVal)
} catch (err) {
const errMsg = `found ipns record that we couldn't convert to a value`

log.error(errMsg)
return callback(errcode(new Error(errMsg), 'ERR_INVALID_IPNS_RECORD'))
log.error(err)
return callback(errcode(new Error(`found ipns record that we couldn't convert to a value`), 'ERR_INVALID_IPNS_RECORD'))
}

callback(null, record.value)
Expand Down
44 changes: 12 additions & 32 deletions src/core/ipns/resolver.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,7 @@ class IpnsResolver {
}

if (typeof name !== 'string') {
const errMsg = `one or more of the provided parameters are not valid`

log.error(errMsg)
return callback(errcode(new Error(errMsg), 'ERR_INVALID_PARAMETER'))
return callback(errcode(new Error('invalid name'), 'ERR_INVALID_NAME'))
}

options = options || {}
Expand All @@ -35,10 +32,7 @@ class IpnsResolver {
const nameSegments = name.split('/')

if (nameSegments.length !== 3 || nameSegments[0] !== '') {
const errMsg = `invalid name syntax for ${name}`

log.error(errMsg)
return callback(errcode(new Error(errMsg), 'ERR_INVALID_NAME_SYNTAX'))
return callback(errcode(new Error('invalid name'), 'ERR_INVALID_NAME'))
}

const key = nameSegments[2]
Expand Down Expand Up @@ -101,27 +95,20 @@ class IpnsResolver {

this._routing.get(routingKey.toBuffer(), (err, record) => {
if (err) {
log.error(err)
if (err.code !== 'ERR_NOT_FOUND') {
const errMsg = `unexpected error getting the ipns record ${peerId.id}`

log.error(errMsg)
return callback(errcode(new Error(errMsg), 'ERR_UNEXPECTED_ERROR_GETTING_RECORD'))
return callback(errcode(new Error(`unexpected error getting the ipns record ${peerId.id}`), 'ERR_UNEXPECTED_ERROR_GETTING_RECORD'))
}
const errMsg = `record requested was not found for ${name} (${routingKey}) in the network`

log.error(errMsg)
return callback(errcode(new Error(errMsg), 'ERR_NO_RECORD_FOUND'))
return callback(errcode(new Error(`record requested was not found for ${name} (${routingKey}) in the network`), 'ERR_NO_RECORD_FOUND'))
}

// IPNS entry
let ipnsEntry
try {
ipnsEntry = ipns.unmarshal(record)
} catch (err) {
const errMsg = `found ipns record that we couldn't convert to a value`

log.error(errMsg)
return callback(errcode(new Error(errMsg), 'ERR_INVALID_RECORD_RECEIVED'))
log.error(err)
return callback(errcode(new Error(`found ipns record that we couldn't convert to a value`), 'ERR_INVALID_RECORD_RECEIVED'))
}

// if the record has a public key validate it
Expand All @@ -132,26 +119,19 @@ class IpnsResolver {
// Otherwise, try to get the public key from routing
this._routing.get(routingKey.toBuffer(), (err, pubKey) => {
if (err) {
log.error(err)
if (err.code !== 'ERR_NOT_FOUND') {
const errMsg = `unexpected error getting the public key for the ipns record ${peerId.id}`

log.error(errMsg)
return callback(errcode(new Error(errMsg), 'ERR_UNEXPECTED_ERROR_GETTING_PUB_KEY'))
return callback(errcode(new Error(`unexpected error getting the public key for the ipns record ${peerId.id}`), 'ERR_UNEXPECTED_ERROR_GETTING_PUB_KEY'))
}
const errMsg = `public key requested was not found for ${name} (${routingPubKey}) in the network`

log.error(errMsg)
return callback(errcode(new Error(errMsg), 'ERR_NO_RECORD_FOUND'))
return callback(errcode(new Error(`public key requested was not found for ${name} (${routingPubKey}) in the network`), 'ERR_NO_RECORD_FOUND'))
}

try {
// Insert it into the peer id, in order to be validated by IPNS validator
peerId.pubKey = crypto.keys.unmarshalPublicKey(pubKey)
} catch (err) {
const errMsg = `found public key record that we couldn't convert to a value`

log.error(errMsg)
return callback(errcode(new Error(errMsg), 'ERR_INVALID_PUB_KEY_RECEIVED'))
log.error(err)
return callback(errcode(new Error(`found public key record that we couldn't convert to a value`), 'ERR_INVALID_PUB_KEY_RECEIVED'))
}

this._validateRecord(peerId, ipnsEntry, callback)
Expand Down
27 changes: 7 additions & 20 deletions src/core/ipns/routing/offline-datastore.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,28 +25,20 @@ class OfflineDatastore {
*/
put (key, value, callback) {
if (!Buffer.isBuffer(key)) {
const errMsg = `Offline datastore key must be a buffer`

log.error(errMsg)
return callback(errcode(new Error(errMsg), 'ERR_INVALID_KEY'))
return callback(errcode(new Error('Offline datastore key must be a buffer'), 'ERR_INVALID_KEY'))
}

if (!Buffer.isBuffer(value)) {
const errMsg = `Offline datastore value must be a buffer`

log.error(errMsg)
return callback(errcode(new Error(errMsg), 'ERR_INVALID_VALUE'))
return callback(errcode(new Error('Offline datastore value must be a buffer'), 'ERR_INVALID_VALUE'))
}

let routingKey

try {
routingKey = this._routingKey(key)
} catch (err) {
const errMsg = `Not possible to generate the routing key`

log.error(errMsg)
return callback(errcode(new Error(errMsg), 'ERR_GENERATING_ROUTING_KEY'))
log.error(err)
return callback(errcode(new Error('Not possible to generate the routing key'), 'ERR_GENERATING_ROUTING_KEY'))
}

// Marshal to libp2p record as the DHT does
Expand All @@ -63,21 +55,16 @@ class OfflineDatastore {
*/
get (key, callback) {
if (!Buffer.isBuffer(key)) {
const errMsg = `Offline datastore key must be a buffer`

log.error(errMsg)
return callback(errcode(new Error(errMsg), 'ERR_INVALID_KEY'))
return callback(errcode(new Error('Offline datastore key must be a buffer'), 'ERR_INVALID_KEY'))
}

let routingKey

try {
routingKey = this._routingKey(key)
} catch (err) {
const errMsg = `Not possible to generate the routing key`

log.error(errMsg)
return callback(errcode(new Error(errMsg), 'ERR_GENERATING_ROUTING_KEY'))
log.error(err)
return callback(errcode(new Error('Not possible to generate the routing key'), 'ERR_GENERATING_ROUTING_KEY'))
}

this._repo.datastore.get(routingKey, (err, res) => {
Expand Down
10 changes: 2 additions & 8 deletions src/core/ipns/routing/pubsub-datastore.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,7 @@ class IpnsPubsubDatastore {
const subscriber = this._subscriptions[key]

if (!subscriber) {
const errMsg = `key ${key} does not correspond to a subscription`

log.error(errMsg)
return callback(errcode(new Error(errMsg), 'ERR_INVALID_KEY'))
return callback(errcode(new Error(`key ${key} does not correspond to a subscription`), 'ERR_INVALID_KEY'))
}

let keys
Expand Down Expand Up @@ -105,10 +102,7 @@ class IpnsPubsubDatastore {
*/
cancel (name, callback) {
if (typeof name !== 'string') {
const errMsg = `received subscription name is not valid`

log.error(errMsg)
return callback(errcode(new Error(errMsg), 'ERR_INVALID_SUBSCRIPTION_NAME'))
return callback(errcode(new Error('invalid subscription name'), 'ERR_INVALID_SUBSCRIPTION_NAME'))
}

// Trim /ipns/ prefix from the name
Expand Down
4 changes: 2 additions & 2 deletions src/core/runtime/dns-nodejs.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ module.exports = (domain, opts, callback) => {

function recursiveResolveDnslink (domain, depth, callback) {
if (depth === 0) {
return callback(errcode(`recursion limit exceeded`, 'ERR_DNSLINK_RECURSION_LIMIT'))
return callback(errcode(new Error('recursion limit exceeded'), 'ERR_DNSLINK_RECURSION_LIMIT'))
}

return resolveDnslink(domain)
Expand Down Expand Up @@ -70,7 +70,7 @@ function resolveDnslink (domain) {
// we now have dns text entries as an array of strings
// only records passing the DNSLINK_REGEX text are included
if (dnslinkRecords.length === 0) {
throw errcode(`No dnslink records found for domain: ${domain}`, 'ERR_DNSLINK_NOT_FOUND')
throw errcode(new Error(`No dnslink records found for domain: ${domain}`), 'ERR_DNSLINK_NOT_FOUND')
}
return dnslinkRecords[0]
})
Expand Down
6 changes: 3 additions & 3 deletions test/core/name.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ describe('name', function () {
it('should error to publish if does not receive private key', function (done) {
node._ipns.publisher.publish(null, ipfsRef, (err) => {
expect(err).to.exist()
expect(err.code).to.equal('ERR_UNDEFINED_PARAMETER')
expect(err.code).to.equal('ERR_INVALID_PRIVATE_KEY')
done()
})
})
Expand Down Expand Up @@ -286,15 +286,15 @@ describe('name', function () {
it('should error to resolve if the received name is not a string', function (done) {
node._ipns.resolver.resolve(false, (err) => {
expect(err).to.exist()
expect(err.code).to.equal('ERR_INVALID_PARAMETER')
expect(err.code).to.equal('ERR_INVALID_NAME')
done()
})
})

it('should error to resolve if receives an invalid ipns path', function (done) {
node._ipns.resolver.resolve('ipns/<cid>', (err) => {
expect(err).to.exist()
expect(err.code).to.equal('ERR_INVALID_NAME_SYNTAX')
expect(err.code).to.equal('ERR_INVALID_NAME')
done()
})
})
Expand Down

0 comments on commit bb4dc19

Please sign in to comment.