From 40202a00b7560f9c1451f39f049ac22f9757e814 Mon Sep 17 00:00:00 2001 From: Feross Aboukhadijeh Date: Wed, 1 Mar 2017 22:54:40 -0800 Subject: [PATCH] BREAKING: change how the filter function works It's non-standard for a callback function to take a non-error argument in the first position. So instead of the filter callback accepting three types of arguments: cb(true) // allowed cb(false) // disallowed cb(new Error('custom message')) // disallowed with custom message It now accepts two forms: cb(new Error('custom message')) // disallowed with custom message cb(null) // allowed --- README.md | 12 ++++++++---- server.js | 9 ++++----- test/filter.js | 17 ++++++++++++----- 3 files changed, 24 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index d834889..4302e1d 100644 --- a/README.md +++ b/README.md @@ -157,10 +157,14 @@ var server = new Server({ // This example only allows one torrent. var allowed = (infoHash === 'aaa67059ed6bd08362da625b3ae77f6f4a075aaa') - cb(allowed) - - // In addition to returning a boolean (`true` for allowed, `false` for disallowed), - // you can return an `Error` object to disallow and provide a custom reason. + if (allowed) { + // If the callback is passed `null`, the torrent will be allowed. + cb(null) + } else { + // If the callback is passed an `Error` object, the torrent will be disallowed + // and the error's `message` property will be given as the reason. + cb(new Error('disallowed torrent')) + } } }) diff --git a/server.js b/server.js index 8548786..3723516 100644 --- a/server.js +++ b/server.js @@ -661,11 +661,10 @@ Server.prototype._onAnnounce = function (params, cb) { function createSwarm () { if (self._filter) { - self._filter(params.info_hash, params, function (allowed) { - if (allowed instanceof Error) { - cb(allowed) - } else if (!allowed) { - cb(new Error('disallowed info_hash')) + self._filter(params.info_hash, params, function (err) { + // Precense of err means that this info_hash is disallowed + if (err) { + cb(err) } else { self.createSwarm(params.info_hash, function (err, swarm) { if (err) return cb(err) diff --git a/test/filter.js b/test/filter.js index ffdd567..8eb4dd8 100644 --- a/test/filter.js +++ b/test/filter.js @@ -12,7 +12,11 @@ function testFilterOption (t, serverType) { var opts = { serverType: serverType } // this is test-suite-only option opts.filter = function (infoHash, params, cb) { process.nextTick(function () { - cb(infoHash !== fixtures.alice.parsedTorrent.infoHash) + if (infoHash === fixtures.alice.parsedTorrent.infoHash) { + cb(new Error('disallowed info_hash (Alice)')) + } else { + cb(null) + } }) } @@ -29,7 +33,7 @@ function testFilterOption (t, serverType) { if (serverType === 'ws') common.mockWebsocketTracker(client1) client1.once('warning', function (err) { - t.ok(/disallowed info_hash/.test(err.message), 'got client warning') + t.ok(err.message.includes('disallowed info_hash (Alice)'), 'got client warning') client1.destroy(function () { t.pass('client1 destroyed') @@ -62,7 +66,7 @@ function testFilterOption (t, serverType) { server.removeAllListeners('warning') server.once('warning', function (err) { - t.ok(/disallowed info_hash/.test(err.message), 'got server warning') + t.ok(err.message.includes('disallowed info_hash (Alice)'), 'got server warning') t.equal(Object.keys(server.torrents).length, 0) }) @@ -88,8 +92,11 @@ function testFilterCustomError (t, serverType) { var opts = { serverType: serverType } // this is test-suite-only option opts.filter = function (infoHash, params, cb) { process.nextTick(function () { - if (infoHash === fixtures.alice.parsedTorrent.infoHash) cb(new Error('alice blocked')) - else cb(true) + if (infoHash === fixtures.alice.parsedTorrent.infoHash) { + cb(new Error('alice blocked')) + } else { + cb(null) + } }) }