nebius/SWE-rebench-V2-PRs huggingface dataset:
Repository: moscajs/aedes
Pull request #407 (2020-02-05 17:16:42)

Not emit clientReady/connected event when error raised in CONNECT phase


Patch:
diff --git a/lib/handlers/connect.js b/lib/handlers/connect.js
index 4022e9c..82a8e9e 100644
--- a/lib/handlers/connect.js
+++ b/lib/handlers/connect.js
@@ -88,8 +88,10 @@ function init (client, packet, done) {
     connectActions,
     { returnCode: 0, sessionPresent: false }, // [MQTT-3.1.4-4], [MQTT-3.2.2-4]
     function (err) {
-      this.client.broker.emit('clientReady', client)
-      this.client.emit('connected')
+      if (!err) {
+        this.client.broker.emit('clientReady', client)
+        this.client.emit('connected')
+      }
       done(err)
     })
 }


Test patch:
diff --git a/test/auth.js b/test/auth.js
index eb31bb6..fc512e2 100644
--- a/test/auth.js
+++ b/test/auth.js
@@ -58,6 +58,10 @@ test('authenticate unsuccessfully a client with username and password', function
     cb(null, false)
   }
 
+  s.broker.on('clientReady', function (client) {
+    t.fail('client should not ready')
+  })
+
   s.outStream.on('data', function (packet) {
     t.deepEqual(packet, {
       cmd: 'connack',
@@ -101,6 +105,10 @@ test('authenticate errors', function (t) {
     cb(new Error('this should happen!'))
   }
 
+  s.broker.on('clientReady', function (client) {
+    t.fail('client should not ready')
+  })
+
   s.outStream.on('data', function (packet) {
     t.deepEqual(packet, {
       cmd: 'connack',
@@ -146,6 +154,10 @@ test('authentication error when return code 1 (unacceptable protocol version) is
     cb(error, null)
   }
 
+  s.broker.on('clientReady', function (client) {
+    t.fail('client should not ready')
+  })
+
   s.outStream.on('data', function (packet) {
     t.deepEqual(packet, {
       cmd: 'connack',
@@ -191,6 +203,10 @@ test('authentication error when return code 2 (identifier rejected) is passed',
     cb(error, null)
   }
 
+  s.broker.on('clientReady', function (client) {
+    t.fail('client should not ready')
+  })
+
   s.outStream.on('data', function (packet) {
     t.deepEqual(packet, {
       cmd: 'connack',
@@ -236,6 +252,10 @@ test('authentication error when return code 3 (Server unavailable) is passed', f
     cb(error, null)
   }
 
+  s.broker.on('clientReady', function (client) {
+    t.fail('client should not ready')
+  })
+
   s.outStream.on('data', function (packet) {
     t.deepEqual(packet, {
       cmd: 'connack',
@@ -281,6 +301,10 @@ test('authentication error when non numeric return code is passed', function (t)
     cb(error, null)
   }
 
+  s.broker.on('clientReady', function (client) {
+    t.fail('client should not ready')
+  })
+
   s.outStream.on('data', function (packet) {
     t.deepEqual(packet, {
       cmd: 'connack',
Repository: moscajs/aedes
Pull request #507 (2020-06-12 07:41:30)

fix: prevent client connection to be closed when doing connack
Could be a possible fix of #483 

coveralls: ## Pull Request Test Coverage Report for [Build 133062303](https://coveralls.io/builds/31409549)

* **0** of **0**   changed or added relevant lines in **0** files are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage decreased (**-0.0004%**) to **99.543%**

---



|  Totals | [![Coverage Status](https://coveralls.io/builds/31409549/badge)](https://coveralls.io/builds/31409549) |
| :-- | --: |
| Change from base [Build 132494017](https://coveralls.io/builds/31397530): |  -0.0004% |
| Covered Lines: | 779 |
| Relevant Lines: | 780 |

---
##### 💛  - [Coveralls](https://coveralls.io)

mcollina: I would add a test. Also, it seems really obscure on why this is the fix.
robertsLando: @mcollina Based on [this](https://github.com/moscajs/aedes/issues/483#issuecomment-642880187) comment I was looking at the connect handler and I found `client.conn.end()` immediatly after the `doConnack` call, this could cause some race condition and the connection could be closed before the connack is sent, also it is useless as when the connack is sent done callback is called with an error and it will be catched by client `nextBatch` (https://github.com/moscajs/aedes/blob/master/lib/client.js#L62) and the connection will be closed
robertsLando: If this seems logic for you now I will write a test for that

Patch:
diff --git a/lib/handlers/connect.js b/lib/handlers/connect.js
index 7b16a29..66c3ba0 100644
--- a/lib/handlers/connect.js
+++ b/lib/handlers/connect.js
@@ -75,7 +75,6 @@ function init (client, packet, done) {
     doConnack(
       { client: client, returnCode: returnCode, sessionPresent: false },
       done.bind(this, error))
-    client.conn.end()
     return
   }
 


Test patch:
diff --git a/test/connect.js b/test/connect.js
index 853cd6c..563d5d3 100644
--- a/test/connect.js
+++ b/test/connect.js
@@ -283,13 +283,27 @@ test('clients with zero-byte clientid and clean=true on MQTT 3.1.1 will get a ge
 
 // [MQTT-3.1.3-7]
 test('reject clients with > 23 clientId length in MQTT 3.1.0', function (t) {
-  t.plan(5)
+  t.plan(7)
 
   const broker = aedes()
   t.tearDown(broker.close.bind(broker))
 
   const s = setup(broker)
 
+  var conn = s.client.conn
+  var end = conn.end
+
+  conn.end = function () {
+    t.fail('should not call `conn.end()`')
+    end()
+  }
+
+  function drain () {
+    t.pass('should empty connection request queue')
+  }
+
+  conn._writableState.getBuffer = () => [{ callback: drain }, { callback: drain }]
+
   s.inStream.write({
     cmd: 'connect',
     protocolId: 'MQIsdp',
Repository: moscajs/aedes
Pull request #652 (2021-07-13 17:56:49)

fix: missed messageId `0`

robertsLando: Please fix lint errors and add a test to cover this

Patch:
diff --git a/lib/handlers/subscribe.js b/lib/handlers/subscribe.js
index 9387623..453007f 100644
--- a/lib/handlers/subscribe.js
+++ b/lib/handlers/subscribe.js
@@ -184,7 +184,7 @@ function completeSubscribe (err) {
   const packet = this.packet
   const client = this.client
 
-  if (packet.messageId) {
+  if (packet.messageId !== undefined) {
     // [MQTT-3.9.3-1]
     write(client,
       new SubAck(packet, this.subState.map(obj => obj.granted)),
@@ -234,6 +234,6 @@ function completeSubscribe (err) {
   }))
 }
 
-function noop () {}
+function noop () { }
 
 module.exports = handleSubscribe
diff --git a/lib/handlers/unsubscribe.js b/lib/handlers/unsubscribe.js
index 2ade345..e08c317 100644
--- a/lib/handlers/unsubscribe.js
+++ b/lib/handlers/unsubscribe.js
@@ -26,7 +26,7 @@ function handleUnsubscribe (client, packet, done) {
     }
   }
 
-  if (packet.messageId) {
+  if (packet.messageId !== undefined) {
     if (client.clean) {
       return actualUnsubscribe(client, packet, done)
     }
@@ -80,7 +80,7 @@ function completeUnsubscribe (err) {
   const packet = this.packet
   const done = this.finish
 
-  if (packet.messageId) {
+  if (packet.messageId !== undefined) {
     write(client, new UnSubAck(packet),
       done)
   } else {
@@ -99,6 +99,6 @@ function completeUnsubscribe (err) {
   }
 }
 
-function noop () {}
+function noop () { }
 
 module.exports = handleUnsubscribe


Test patch:
diff --git a/test/basic.js b/test/basic.js
index 92b2579..81130a3 100644
--- a/test/basic.js
+++ b/test/basic.js
@@ -260,6 +260,35 @@ test('subscribe should have messageId', function (t) {
   })
 })
 
+test('subscribe with messageId 0 should return suback', function (t) {
+  t.plan(1)
+
+  const s = connect(setup())
+  t.teardown(s.broker.close.bind(s.broker))
+
+  s.inStream.write({
+    cmd: 'subscribe',
+    subscriptions: [{
+      topic: 'hello',
+      qos: 0
+    }],
+    messageId: 0
+  })
+  s.outStream.once('data', function (packet) {
+    t.same(packet, {
+      cmd: 'suback',
+      messageId: 0,
+      dup: false,
+      length: 3,
+      qos: 0,
+      retain: false,
+      granted: [
+        0
+      ]
+    }, 'packet matches')
+  })
+})
+
 test('unsubscribe', function (t) {
   t.plan(5)
Repository: moscajs/aedes
Pull request #929 (2024-01-08 17:02:14)

fix: delete already closed client on closeSameClients

Gianluca-Casagrande-Stiga: This should resolve https://github.com/moscajs/aedes/issues/928, but I would like to add a related test
coveralls: ## Pull Request Test Coverage Report for [Build 7458304179](https://coveralls.io/builds/64955152)


* **0** of **0**   changed or added relevant lines in **0** files are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage decreased (**-0.3%**) to **99.481%**

---



|  Totals | [![Coverage Status](https://coveralls.io/builds/64955152/badge)](https://coveralls.io/builds/64955152) |
| :-- | --: |
| Change from base [Build 7207966564](https://coveralls.io/builds/64564431): |  -0.3% |
| Covered Lines: | 817 |
| Relevant Lines: | 819 |

---
##### 💛  - [Coveralls](https://coveralls.io)


Patch:
diff --git a/aedes.js b/aedes.js
index fd4f9b7..956b740 100644
--- a/aedes.js
+++ b/aedes.js
@@ -170,7 +170,13 @@ function Aedes (opts) {
     const clientId = packet.payload.toString()
 
     if (that.clients[clientId] && serverId !== that.id) {
-      that.clients[clientId].close(done)
+      if (that.clients[clientId].closed) {
+        // remove the client from the list if it is already closed
+        delete that.clients[clientId]
+        done()
+      } else {
+        that.clients[clientId].close(done)
+      }
     } else {
       done()
     }


Test patch:
diff --git a/test/events.js b/test/events.js
index 4019139..cd2137a 100644
--- a/test/events.js
+++ b/test/events.js
@@ -221,3 +221,20 @@ test('Test backpressure aedes published function', function (t) {
     })
   })
 })
+
+test('clear closed clients when the same clientId is managed by another broker', function (t) {
+  t.plan(1)
+
+  const clientId = 'closed-client'
+  const broker = aedes()
+
+  // simulate a closed client on the broker
+  broker.clients[clientId] = { closed: true }
+
+  // simulate the creation of the same client on another broker of the cluster
+  broker.publish({ topic: '$SYS/anotherbroker/new/clients', payload: clientId }, () => {
+    t.equal(broker.clients[clientId], undefined) // check that the closed client was removed
+  })
+
+  t.teardown(broker.close.bind(broker))
+})