From 1de2c7a0537de3aa4f969f84a696044e1238201f Mon Sep 17 00:00:00 2001 From: Nick Krecklow Date: Sun, 10 May 2020 20:19:01 -0500 Subject: [PATCH 1/8] disable logToDatabase by default --- config.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config.json b/config.json index 157ed92..e96bca1 100644 --- a/config.json +++ b/config.json @@ -13,7 +13,7 @@ "skipUnfurlSrv": false, "unfurlSrvCacheTtl": 120000 }, - "logToDatabase": true, + "logToDatabase": false, "graphDuration": 86400000, "serverGraphDuration": 180000 } From 005798aa0da413021fce0a8a959153eb42ddb077 Mon Sep 17 00:00:00 2001 From: Nick Krecklow Date: Wed, 13 May 2020 16:40:12 -0500 Subject: [PATCH 2/8] fix wrong SRV record name -> _minecraft._tcp. --- lib/servers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/servers.js b/lib/servers.js index 4f06507..211ae91 100644 --- a/lib/servers.js +++ b/lib/servers.js @@ -339,7 +339,7 @@ class ServerRegistration { if (!this._isUnfurlingSrv) { this._isUnfurlingSrv = true - dns.resolveSrv('_minecraft._tcp' + this.data.ip, (_, records) => { + dns.resolveSrv('_minecraft._tcp.' + this.data.ip, (_, records) => { this._lastUnfurlSrv = { timestamp } From a308eb35fb60faaaf396ab94decfb58353b04930 Mon Sep 17 00:00:00 2001 From: Nick Krecklow Date: Wed, 13 May 2020 16:41:05 -0500 Subject: [PATCH 3/8] 5.4.2 release --- docs/CHANGELOG.md | 3 +++ package-lock.json | 2 +- package.json | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 1bf96f4..19b5be5 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -1,3 +1,6 @@ +**5.4.2** *(May 13 2020)* +- Fixes a typo causing `_minecraft._tcp.*` SRV records to not resolve. + **5.4.1** *(May 10 2020)* - Adds warnings when the system is pinging more frequently than it is getting replies. - Replaces the legacy mc-ping-updated dependency with a new library, mcping-js. This fixes some bugs that could cause "zombie" connections and cause stuttering in the ping loops. diff --git a/package-lock.json b/package-lock.json index 6d19366..cef3b35 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "minetrack", - "version": "5.4.1", + "version": "5.4.2", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index 633cb96..42ecbd9 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "minetrack", - "version": "5.4.1", + "version": "5.4.2", "description": "A Minecraft server tracker that lets you focus on the basics.", "main": "main.js", "dependencies": { From 9a38160019f9e90ac3af5b4a3a9d269cbe955686 Mon Sep 17 00:00:00 2001 From: Nick Krecklow Date: Thu, 14 May 2020 20:30:40 -0500 Subject: [PATCH 4/8] replace ServerRegistration#unfurlSrv to DNSResolver class --- config.json | 4 --- lib/database.js | 4 ++- lib/dns.js | 69 +++++++++++++++++++++++++++++++++++++++++++++++++ lib/logger.js | 2 +- lib/ping.js | 4 +-- lib/servers.js | 59 ++---------------------------------------- lib/time.js | 6 ++++- main.js | 9 ------- 8 files changed, 82 insertions(+), 75 deletions(-) create mode 100644 lib/dns.js diff --git a/config.json b/config.json index e96bca1..ebe2d93 100644 --- a/config.json +++ b/config.json @@ -9,10 +9,6 @@ "pingAll": 3000, "connectTimeout": 2500 }, - "performance": { - "skipUnfurlSrv": false, - "unfurlSrvCacheTtl": 120000 - }, "logToDatabase": false, "graphDuration": 86400000, "serverGraphDuration": 180000 diff --git a/lib/database.js b/lib/database.js index 60f795a..16e5190 100644 --- a/lib/database.js +++ b/lib/database.js @@ -1,5 +1,7 @@ const sqlite = require('sqlite3') +const TimeTracker = require('./time') + class Database { constructor (app) { this._app = app @@ -16,7 +18,7 @@ class Database { loadGraphPoints (graphDuration, callback) { // Query recent pings - const endTime = new Date().getTime() + const endTime = TimeTracker.getEpochMillis() const startTime = endTime - graphDuration this.getRecentPings(startTime, endTime, pingData => { diff --git a/lib/dns.js b/lib/dns.js new file mode 100644 index 0000000..e2ab8ff --- /dev/null +++ b/lib/dns.js @@ -0,0 +1,69 @@ +const dns = require('dns') + +const logger = require('./logger') +const TimeTracker = require('./time') + +const config = require('../config') + +const SKIP_SRV_TIMEOUT = config.skipSrvTimeout || 60 * 60 * 1000 + +class DNSResolver { + constructor (ip, port) { + this._ip = ip + this._port = port + } + + _skipSrv () { + this._skipSrvUntil = TimeTracker.getEpochMillis() + SKIP_SRV_TIMEOUT + } + + _isSkipSrv () { + return this._skipSrvUntil && TimeTracker.getEpochMillis() <= this._skipSrvUntil + } + + resolve (callback) { + if (this._isSkipSrv()) { + callback(this._ip, this._port, config.rates.connectTimeout) + + return + } + + const startTime = TimeTracker.getEpochMillis() + + let callbackFired = false + + const fireCallback = (ip, port) => { + if (!callbackFired) { + callbackFired = true + + // Send currentTime - startTime to provide remaining connectionTime allowance + callback(ip || this._ip, port || this._port, TimeTracker.getEpochMillis() - startTime) + } + } + + const timeoutCallback = setTimeout(fireCallback, config.rates.connectTimeout) + + dns.resolveSrv('_minecraft._tcp.' + this._ip, (err, records) => { + // Cancel the timeout handler if not already fired + if (!callbackFired) { + clearTimeout(timeoutCallback) + } + + // Test if the error indicates a miss, or if the records returned are empty + if ((err && (err.code === 'ENOTFOUND' || err.code === 'ENODATA')) || !records || records.length === 0) { + if (!this._isSkipSrv()) { + this._skipSrv() + + logger.log('warn', 'No SRV records were resolved for %s. Minetrack will skip attempting to resolve %s SRV records for %d minutes.', this._ip, this._ip, SKIP_SRV_TIMEOUT / (60 * 1000)) + } + + fireCallback() + } else { + // Only fires if !err && records.length > 0 + fireCallback(records[0].name, records[0].port) + } + }) + } +} + +module.exports = DNSResolver diff --git a/lib/logger.js b/lib/logger.js index ea99dd7..82237f6 100644 --- a/lib/logger.js +++ b/lib/logger.js @@ -9,7 +9,7 @@ winston.add(winston.transports.File, { winston.add(winston.transports.Console, { timestamp: () => { const date = new Date() - return date.toLocaleTimeString() + ' ' + date.getDate() + '/' + (date.getMonth() + 1) + '/' + date.getFullYear().toString().substring(2, 4) + return date.toLocaleTimeString() + ' ' + date.toLocaleDateString() }, colorize: true }) diff --git a/lib/ping.js b/lib/ping.js index 82d7e33..12901a0 100644 --- a/lib/ping.js +++ b/lib/ping.js @@ -9,10 +9,10 @@ const config = require('../config') function ping (serverRegistration, timeout, callback, version) { switch (serverRegistration.data.type) { case 'PC': - serverRegistration.unfurlSrv((host, port) => { + serverRegistration.dnsResolver.unfurlSrv((host, port, remainingTimeout) => { const server = new minecraftJavaPing.MinecraftServer(host, port || 25565) - server.ping(timeout, version, (err, res) => { + server.ping(remainingTimeout, version, (err, res) => { if (err) { callback(err) } else { diff --git a/lib/servers.js b/lib/servers.js index 211ae91..d6231a6 100644 --- a/lib/servers.js +++ b/lib/servers.js @@ -1,6 +1,6 @@ const crypto = require('crypto') -const dns = require('dns') +const DNSResolver = require('./dns') const TimeTracker = require('./time') const Server = require('./server') @@ -18,6 +18,7 @@ class ServerRegistration { this.serverId = serverId this.data = data this._pingHistory = [] + this.dnsResolver = new DNSResolver(this.data.ip, this.data.port) } handlePing (timestamp, resp, err, version) { @@ -310,62 +311,6 @@ class ServerRegistration { color: this.data.color } } - - unfurlSrv (callback) { - // Skip unfurling SRV, instantly return pre-configured data - if (config.performance && config.performance.skipUnfurlSrv) { - callback(this.data.ip, this.data.port) - return - } - - const timestamp = new Date().getTime() - - // If a cached copy exists and is within its TTL, instantly return - if (this._lastUnfurlSrv && timestamp - this._lastUnfurlSrv.timestamp <= config.performance.unfurlSrvCacheTtl) { - callback(this._lastUnfurlSrv.ip, this._lastUnfurlSrv.port) - return - } - - // Group callbacks into an array - // Once resolved, fire callbacks sequentially - // This avoids callbacks possibly executing out of order - if (!this._unfurlSrvCallbackQueue) { - this._unfurlSrvCallbackQueue = [] - } - - this._unfurlSrvCallbackQueue.push(callback) - - // Prevent multiple #resolveSrv calls per ServerRegistration - if (!this._isUnfurlingSrv) { - this._isUnfurlingSrv = true - - dns.resolveSrv('_minecraft._tcp.' + this.data.ip, (_, records) => { - this._lastUnfurlSrv = { - timestamp - } - - if (records && records.length > 0) { - this._lastUnfurlSrv.ip = records[0].name - this._lastUnfurlSrv.port = records[0].port - } else { - // Provide fallback to pre-configured data - this._lastUnfurlSrv.ip = this.data.ip - this._lastUnfurlSrv.port = this.data.port - } - - // Fire the waiting callbacks in queue - // Release blocking flag to allow new #resolveSrv calls - this._isUnfurlingSrv = false - - for (const callback of this._unfurlSrvCallbackQueue) { - callback(this._lastUnfurlSrv.ip, this._lastUnfurlSrv.port) - } - - // Reset the callback queue - this._unfurlSrvCallbackQueue = [] - }) - } - } } module.exports = ServerRegistration diff --git a/lib/time.js b/lib/time.js index 966272a..57386e2 100644 --- a/lib/time.js +++ b/lib/time.js @@ -7,7 +7,7 @@ class TimeTracker { } newTimestamp () { - const timestamp = new Date().getTime() + const timestamp = TimeTracker.getEpochMillis() this._points.push(timestamp) @@ -22,6 +22,10 @@ class TimeTracker { return this._points } + static getEpochMillis () { + return new Date().getTime() + } + static getMaxServerGraphDataLength () { return Math.ceil(config.serverGraphDuration / config.rates.pingAll) } diff --git a/main.js b/main.js index e0518f7..968df18 100644 --- a/main.js +++ b/main.js @@ -30,15 +30,6 @@ if (!config.serverGraphDuration) { config.serverGraphDuration = 3 * 60 * 10000 } -if (config.performance && config.performance.skipUnfurlSrv) { - logger.log('warn', '"performance.skipUnfurlSrv" is enabled. Any configured hosts using SRV records may not properly resolve.') -} - -if (!config.performance || typeof config.performance.unfurlSrvCacheTtl === 'undefined') { - logger.log('warn', '"performance.unfurlSrvCacheTtl" is not defined in config.json - defaulting to 120 seconds!') - config.performance.unfurlSrvCacheTtl = 2 * 60 * 1000 -} - if (!config.logToDatabase) { logger.log('warn', 'Database logging is not enabled. You can enable it by setting "logToDatabase" to true in config.json. This requires sqlite3 to be installed.') From c516b7c2161a2ed7b1fa0ee12ccdbdceab7f23a7 Mon Sep 17 00:00:00 2001 From: Nick Krecklow Date: Thu, 14 May 2020 20:38:17 -0500 Subject: [PATCH 5/8] release 5.4.3 --- docs/CHANGELOG.md | 4 ++++ lib/dns.js | 3 ++- package-lock.json | 2 +- package.json | 2 +- 4 files changed, 8 insertions(+), 3 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 19b5be5..d079175 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -1,3 +1,7 @@ +**5.4.3** *(May 14 2020)* +- Added support for the optional field `config->skipSrvTimeout` in `config.json`. If a configured server does not return a valid response when unfurling potential SRV records, it will avoid re-unfurling SRV records for this duration in milliseconds. Use a value of `0` to disable this feature altogether. +- Removes support for the `config->performance->skipUnfurlSrv` and `config->performance->unfurlSrvCacheTtl` fields in `config.json + **5.4.2** *(May 13 2020)* - Fixes a typo causing `_minecraft._tcp.*` SRV records to not resolve. diff --git a/lib/dns.js b/lib/dns.js index e2ab8ff..f8bb69b 100644 --- a/lib/dns.js +++ b/lib/dns.js @@ -18,7 +18,8 @@ class DNSResolver { } _isSkipSrv () { - return this._skipSrvUntil && TimeTracker.getEpochMillis() <= this._skipSrvUntil + const ignoreSkipSrv = typeof config.skipSrvTimeout === 'number' && config.skipSrvTimeout <= 0 + return !ignoreSkipSrv && this._skipSrvUntil && TimeTracker.getEpochMillis() <= this._skipSrvUntil } resolve (callback) { diff --git a/package-lock.json b/package-lock.json index cef3b35..7b38dc0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "minetrack", - "version": "5.4.2", + "version": "5.4.3", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index 42ecbd9..4fb1d4b 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "minetrack", - "version": "5.4.2", + "version": "5.4.3", "description": "A Minecraft server tracker that lets you focus on the basics.", "main": "main.js", "dependencies": { From e756584837b35d59a29d78ef41b1793c2d2ea3af Mon Sep 17 00:00:00 2001 From: Nick Krecklow Date: Thu, 14 May 2020 20:43:14 -0500 Subject: [PATCH 6/8] optimize skipSrvTimeout handling --- lib/dns.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/dns.js b/lib/dns.js index f8bb69b..c177284 100644 --- a/lib/dns.js +++ b/lib/dns.js @@ -52,7 +52,12 @@ class DNSResolver { // Test if the error indicates a miss, or if the records returned are empty if ((err && (err.code === 'ENOTFOUND' || err.code === 'ENODATA')) || !records || records.length === 0) { - if (!this._isSkipSrv()) { + // Compare config.skipSrvTimeout directly since SKIP_SRV_TIMEOUT has an or'd value + const isSkipSrvTimeoutActive = typeof config.skipSrvTimeout !== 'number' || config.skipSrvTimeout > 0 + + // Only activate _skipSrv if the skipSrvTimeout value is either NaN or > 0 + // 0 represents a disabled flag + if (!this._isSkipSrv() && !isSkipSrvTimeoutActive) { this._skipSrv() logger.log('warn', 'No SRV records were resolved for %s. Minetrack will skip attempting to resolve %s SRV records for %d minutes.', this._ip, this._ip, SKIP_SRV_TIMEOUT / (60 * 1000)) From 70cfa641e6bbc0ba36969bbaae7d6c31b6d47cbe Mon Sep 17 00:00:00 2001 From: Nick Krecklow Date: Thu, 14 May 2020 20:43:55 -0500 Subject: [PATCH 7/8] remove legacy _isSkipSrv logic --- lib/dns.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/dns.js b/lib/dns.js index c177284..b96ca16 100644 --- a/lib/dns.js +++ b/lib/dns.js @@ -18,8 +18,7 @@ class DNSResolver { } _isSkipSrv () { - const ignoreSkipSrv = typeof config.skipSrvTimeout === 'number' && config.skipSrvTimeout <= 0 - return !ignoreSkipSrv && this._skipSrvUntil && TimeTracker.getEpochMillis() <= this._skipSrvUntil + return this._skipSrvUntil && TimeTracker.getEpochMillis() <= this._skipSrvUntil } resolve (callback) { From 05487d15ca4bde83ebfa40b3f351e12bf738dc59 Mon Sep 17 00:00:00 2001 From: Nick Krecklow Date: Thu, 14 May 2020 20:46:08 -0500 Subject: [PATCH 8/8] reduce naming inversion to simplify logic reading --- lib/dns.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/dns.js b/lib/dns.js index b96ca16..a0b7332 100644 --- a/lib/dns.js +++ b/lib/dns.js @@ -52,11 +52,12 @@ class DNSResolver { // Test if the error indicates a miss, or if the records returned are empty if ((err && (err.code === 'ENOTFOUND' || err.code === 'ENODATA')) || !records || records.length === 0) { // Compare config.skipSrvTimeout directly since SKIP_SRV_TIMEOUT has an or'd value - const isSkipSrvTimeoutActive = typeof config.skipSrvTimeout !== 'number' || config.skipSrvTimeout > 0 + // isValidSkipSrvTimeout == whether the config has a valid skipSrvTimeout value set + const isValidSkipSrvTimeout = typeof config.skipSrvTimeout === 'number' && config.skipSrvTimeout > 0 // Only activate _skipSrv if the skipSrvTimeout value is either NaN or > 0 // 0 represents a disabled flag - if (!this._isSkipSrv() && !isSkipSrvTimeoutActive) { + if (!this._isSkipSrv() && isValidSkipSrvTimeout) { this._skipSrv() logger.log('warn', 'No SRV records were resolved for %s. Minetrack will skip attempting to resolve %s SRV records for %d minutes.', this._ip, this._ip, SKIP_SRV_TIMEOUT / (60 * 1000))