fix: redis promise syntax for tls & ob/tls (#3064)

* bump plugin versions * chore(ci): merge tests into one file * chore(dep): replace included aliases with npm package * chore(dep): bump pi-watch to 2.0.1 * chore(cov): config updates* tls: redis.get is now async * more redis promise updates * fix: dns_list_base: redis.hset -> hSet
haraka · May 28, 2022 · 2bd2682 · 2bd2682
1 parent a8d89ca
commit 2bd2682
Show file tree

Hide file tree

Showing 8 changed files with 43 additions and 543 deletions.
diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml
@@ -11,7 +11,6 @@ jobs:
     runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@v3
-
     - uses: actions/setup-node@v3
       name: Node.js ${{ env.node_version }}
       with:
@@ -27,7 +26,7 @@ jobs:
     - name: run coverage
       run: |
         npm install --no-save c8
-        npx c8 --reporter=lcovonly npm run test
+        npx c8 --reporter=lcovonly npm test
       env:
         NODE_ENV: cov
 

diff --git a/Changes.md b/Changes.md
@@ -3,16 +3,14 @@
 
 ### Changes
 
-
 - dkim_sign: reformat dkim signature to multi-line #2991
 - dkim_sign: remove spurious error logging #3034
-- chore: add lots of `if (!transaction) return` in places #2732
 - doc(queue.js) spelling & grammar improvement #3051
 - doc(rails): add haraka-plugin-queue-rails #2995
 - doc(smtp.ini): correct spelling of SMTPUTF8 #2993
 - style(es6): use optional chaining when accessing transactions #2732
 - style(smtp_client): pass args as objects (was positional)
-- style(plugin/*): transaction guarding #3032
+- style(plugin/\*): transaction guarding #3032
 - dep(generic-pool): 2.5 -> 3.8 (promises) #3033, #3060
 - dep(redis): 3.1 -> 4.1 #3058
 - dep(haraka-plugin-redis): 1.0 -> 2.0  #3038
@@ -25,8 +23,9 @@
 - fix(helo): remove multi-check from should_skip #3041
 - fix(outbound): outbound local mx check #3010
 - fix(outbound): prevent delivery loop when target MX resolves to local hostname #3002
-- test(windows): build shims for windows-2022 & node on windows #3052
-- test: restore CI tests to working order #3030
+- chore: add lots of `if (!transaction) return` in places #2732
+- chore(test): build shims for windows-2022 & node on windows #3052
+- chore(test): restore CI tests to working order #3030
 
 
 ## 2.8.28 - 2021-10-14

diff --git a/outbound/tls.js b/outbound/tls.js
@@ -78,14 +78,14 @@ class OutboundTLS {
         if (!obtls.cfg.redis.disable_for_failed_hosts) return cb_ok();
 
         const dbkey = `no_tls|${host}`;
-        obtls.db.get(dbkey, (err, dbr) => {
-            if (err) {
+        obtls.db.get(dbkey)
+            .then(dbr => {
+                dbr ? cb_nogo(dbr) : cb_ok();
+            })
+            .catch(err => {
                 obtls.logdebug(obtls, `Redis returned error: ${err}`);
-                return cb_ok();
-            }
-
-            return dbr ? cb_nogo(dbr) : cb_ok();
-        });
+                cb_ok();
+            })
     }
 
     mark_tls_nogo (host, cb) {
@@ -97,10 +97,11 @@ class OutboundTLS {
 
         logger.lognotice(obtls, `TLS connection failed. Marking ${host} as non-TLS for ${expiry} seconds`);
 
-        obtls.db.setex(dbkey, expiry, (new Date()).toISOString(), (err, dbr) => {
-            if (err) logger.logerror(obtls, `Redis returned error: ${err}`);
-            cb();
-        });
+        obtls.db.setex(dbkey, expiry, (new Date()).toISOString())
+            .then(cb)
+            .catch(err => {
+                logger.logerror(obtls, `Redis returned error: ${err}`);
+            })
     }
 }
 

diff --git a/package.json b/package.json
@@ -39,7 +39,7 @@
     "haraka-dsn"            : "^1.0.3",
     "haraka-net-utils"      : "^1.3.3",
     "haraka-notes"          : "^1.0.4",
-    "haraka-plugin-redis"   : "^2.0.3",
+    "haraka-plugin-redis"   : "^2.0.5",
     "haraka-results"        : "^2.2.0",
     "haraka-tld"            : "^1.0.30",
     "haraka-utils"          : "^1.0.2",
@@ -48,6 +48,7 @@
   },
   "optionalDependencies": {
     "haraka-plugin-access"  : "^1.1.4",
+    "haraka-plugin-aliases" : "^1.0.1",
     "haraka-plugin-asn"     : "^2.0.0",
     "haraka-plugin-auth-ldap": "^1.0.2",
     "haraka-plugin-dcc"     : "^1.0.1",
@@ -64,7 +65,7 @@
     "haraka-plugin-recipient-routes" : "^1.0.2",
     "haraka-plugin-rspamd"  : "^1.1.6",
     "haraka-plugin-syslog"  : "^1.0.3",
-    "haraka-plugin-watch"   : "^2.0.0",
+    "haraka-plugin-watch"   : "^2.0.1",
     "ocsp"                  : "~1.2.0",
     "redis"                 : "~4.1.0",
     "tmp"                   : "~0.2.1"

diff --git a/plugins/aliases.js b/plugins/aliases.js
diff --git a/plugins/dns_list_base.js b/plugins/dns_list_base.js
@@ -68,14 +68,13 @@ exports.stats_incr_zone = function (err, zone, start) {
 
     const rkey = `dns-list-stat:${zone}`;
     const elapsed = new Date().getTime() - start;
-    redis_client.hincrby(rkey, 'TOTAL', 1);
+    redis_client.hIncrBy(rkey, 'TOTAL', 1);
     const foo = (err) ? err.code : 'LISTED';
-    redis_client.hincrby(rkey, foo, 1);
-    redis_client.hget(rkey, 'AVG_RT', (err2, rt) => {
-        if (err2) return;
+    redis_client.hIncrBy(rkey, foo, 1);
+    redis_client.hGet(rkey, 'AVG_RT').then(rt => {
         const avg = parseInt(rt) ? (parseInt(elapsed) + parseInt(rt))/2
             : parseInt(elapsed);
-        redis_client.hset(rkey, 'AVG_RT', avg);
+        redis_client.hSet(rkey, 'AVG_RT', avg);
     });
 }
 
@@ -89,12 +88,14 @@ exports.init_redis = function () {
     const port = parseInt(host_port[1], 10) || 6379;
 
     redis_client = redis.createClient(port, host);
-    redis_client.on('error', err => {
-        plugin.logerror(`Redis error: ${err}`);
-        redis_client.quit();
-        redis_client = null; // should force a reconnect
-        // not sure if that's the right thing but better than nothing...
-    });
+    redis_client.connect().then(() => {
+        redis_client.on('error', err => {
+            plugin.logerror(`Redis error: ${err}`);
+            redis_client.quit();
+            redis_client = null; // should force a reconnect
+            // not sure if that's the right thing but better than nothing...
+        })
+    })
 }
 
 exports.multi = function (lookup, zones, cb) {
@@ -110,7 +111,7 @@ exports.multi = function (lookup, zones, cb) {
         // Statistics: check hit overlap
         for (let i=0; i < listed.length; i++) {
             const foo = (listed[i] === zone) ? 'TOTAL' : listed[i];
-            redis_client.hincrby(`dns-list-overlap:${zone}`, foo, 1);
+            redis_client.hIncrBy(`dns-list-overlap:${zone}`, foo, 1);
         }
     }
 
@@ -194,9 +195,7 @@ exports.check_zones = function (interval) {
 
 exports.shutdown = function () {
     clearInterval(this._interval);
-    if (redis_client) {
-        redis_client.quit();
-    }
+    if (redis_client) redis_client.quit();
 }
 
 exports.disable_zone = function (zone, result) {

diff --git a/plugins/tls.js b/plugins/tls.js
@@ -55,17 +55,16 @@ exports.advertise_starttls = function (next, connection) {
     const redis = server.notes.redis;
     const dbkey = `no_tls|${connection.remote.ip}`;
 
-    redis.get(dbkey, (err, dbr) => {
-        if (err) {
+    redis.get(dbkey)
+        .then(dbr => {
+            if (!dbr) return enable_tls();
+            connection.results.add(plugin, { msg: 'no_tls'});
+            next(CONT, 'STARTTLS disabled because previous attempt failed')
+        })
+        .catch(err => {
             connection.results.add(plugin, {err});
-            return enable_tls();
-        }
-
-        if (!dbr) return enable_tls();
-
-        connection.results.add(plugin, { msg: 'no_tls'});
-        return next(CONT, 'STARTTLS disabled because previous attempt failed')
-    });
+            enable_tls();
+        })
 }
 
 exports.set_notls = function (connection) {