فهرست منبع

[misc] wrap redis errors as tagging does not work with them

ioredis may reuse the error instance for multiple callbacks. E.g. when
 the connection to redis fails, the queue is flushed with the same
 MaxRetriesPerRequestError instance.
Jakob Ackermann 6 سال پیش
والد
کامیت
ee3d3b09ed

+ 10 - 8
services/real-time/app/js/ChannelManager.js

@@ -23,12 +23,13 @@ module.exports = {
     const channel = `${baseChannel}:${id}`
     const channel = `${baseChannel}:${id}`
     const actualSubscribe = function () {
     const actualSubscribe = function () {
       // subscribe is happening in the foreground and it should reject
       // subscribe is happening in the foreground and it should reject
-      const p = rclient.subscribe(channel)
-      p.finally(function () {
-        if (clientChannelMap.get(channel) === subscribePromise) {
-          clientChannelMap.delete(channel)
-        }
-      })
+      return rclient
+        .subscribe(channel)
+        .finally(function () {
+          if (clientChannelMap.get(channel) === subscribePromise) {
+            clientChannelMap.delete(channel)
+          }
+        })
         .then(function () {
         .then(function () {
           logger.log({ channel }, 'subscribed to channel')
           logger.log({ channel }, 'subscribed to channel')
           metrics.inc(`subscribe.${baseChannel}`)
           metrics.inc(`subscribe.${baseChannel}`)
@@ -37,9 +38,10 @@ module.exports = {
           logger.error({ channel, err }, 'failed to subscribe to channel')
           logger.error({ channel, err }, 'failed to subscribe to channel')
           metrics.inc(`subscribe.failed.${baseChannel}`)
           metrics.inc(`subscribe.failed.${baseChannel}`)
           // add context for the stack-trace at the call-site
           // add context for the stack-trace at the call-site
-          OError.tag(err, 'failed to subscribe to channel', { channel })
+          throw new OError('failed to subscribe to channel', {
+            channel
+          }).withCause(err)
         })
         })
-      return p
     }
     }
 
 
     const pendingActions = clientChannelMap.get(channel) || Promise.resolve()
     const pendingActions = clientChannelMap.get(channel) || Promise.resolve()

+ 5 - 5
services/real-time/app/js/ConnectedUsersManager.js

@@ -68,7 +68,7 @@ module.exports = {
 
 
     multi.exec(function (err) {
     multi.exec(function (err) {
       if (err) {
       if (err) {
-        OError.tag(err, 'problem marking user as connected')
+        err = new OError('problem marking user as connected').withCause(err)
       }
       }
       callback(err)
       callback(err)
     })
     })
@@ -104,7 +104,7 @@ module.exports = {
     multi.del(Keys.connectedUser({ project_id, client_id }))
     multi.del(Keys.connectedUser({ project_id, client_id }))
     multi.exec(function (err) {
     multi.exec(function (err) {
       if (err) {
       if (err) {
-        OError.tag(err, 'problem marking user as disconnected')
+        err = new OError('problem marking user as disconnected').withCause(err)
       }
       }
       callback(err)
       callback(err)
     })
     })
@@ -116,9 +116,9 @@ module.exports = {
       result
       result
     ) {
     ) {
       if (err) {
       if (err) {
-        OError.tag(err, 'problem fetching connected user details', {
+        err = new OError('problem fetching connected user details', {
           other_client_id: client_id
           other_client_id: client_id
-        })
+        }).withCause(err)
         return callback(err)
         return callback(err)
       }
       }
       if (!(result && result.user_id)) {
       if (!(result && result.user_id)) {
@@ -154,7 +154,7 @@ module.exports = {
       results
       results
     ) {
     ) {
       if (err) {
       if (err) {
-        OError.tag(err, 'problem getting clients in project')
+        err = new OError('problem getting clients in project').withCause(err)
         return callback(err)
         return callback(err)
       }
       }
       const jobs = results.map((client_id) => (cb) =>
       const jobs = results.map((client_id) => (cb) =>

+ 2 - 2
services/real-time/app/js/DocumentUpdaterManager.js

@@ -136,12 +136,12 @@ const DocumentUpdaterManager = {
       error
       error
     ) {
     ) {
       if (error) {
       if (error) {
-        OError.tag(error, 'error pushing update into redis')
+        error = new OError('error pushing update into redis').withCause(error)
         return callback(error)
         return callback(error)
       }
       }
       rclient.rpush('pending-updates-list', doc_key, function (error) {
       rclient.rpush('pending-updates-list', doc_key, function (error) {
         if (error) {
         if (error) {
-          OError.tag(error, 'error pushing doc_id into redis')
+          error = new OError('error pushing doc_id into redis').withCause(error)
         }
         }
         callback(error)
         callback(error)
       })
       })

+ 2 - 1
services/real-time/test/unit/js/ChannelManagerTests.js

@@ -91,7 +91,8 @@ describe('ChannelManager', function () {
         )
         )
         p.then(() => done(new Error('should not subscribe but fail'))).catch(
         p.then(() => done(new Error('should not subscribe but fail'))).catch(
           (err) => {
           (err) => {
-            err.message.should.equal('some redis error')
+            err.message.should.equal('failed to subscribe to channel')
+            err.cause.message.should.equal('some redis error')
             this.ChannelManager.getClientMapEntry(this.rclient)
             this.ChannelManager.getClientMapEntry(this.rclient)
               .has('applied-ops:1234567890abcdef')
               .has('applied-ops:1234567890abcdef')
               .should.equal(false)
               .should.equal(false)