Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: suback Error Codes Handling #1887

Merged
merged 12 commits into from
Jun 18, 2024
9 changes: 9 additions & 0 deletions DEVELOPMENT.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,12 @@ node -r esbuild-register --test --test-name-pattern="should resend in-flight QoS
```

You can also run tests in watch mode using the `--watch` flag.

## Lint

You can run and automatically fix linting issues with

```sh
npm run lint-fix
```

10 changes: 7 additions & 3 deletions src/lib/handlers/ack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ const handleAck: PacketHandler = (client, packet) => {
const type = packet.cmd
let response = null
const cb = client.outgoing[messageId] ? client.outgoing[messageId].cb : null
let err
let err = null

// Checking `!cb` happens to work, but it's not technically "correct".
//
Expand Down Expand Up @@ -117,7 +117,11 @@ const handleAck: PacketHandler = (client, packet) => {
client.messageIdProvider.deallocate(messageId)
const granted = packet.granted as number[]
for (let grantedI = 0; grantedI < granted.length; grantedI++) {
if ((granted[grantedI] & 0x80) !== 0) {
const subackRC = granted[grantedI]
if ((subackRC & 0x80) !== 0) {
err = new Error(`Subscribe error: ${ReasonCodes[subackRC]}`)
err.code = subackRC

// suback with Failure status
const topics = client.messageIdToTopic[messageId]
if (topics) {
Expand All @@ -129,7 +133,7 @@ const handleAck: PacketHandler = (client, packet) => {
}
delete client.messageIdToTopic[messageId]
client['_invokeStoreProcessingQueue']()
cb(null, packet)
cb(err, packet)
break
}
case 'unsuback': {
Expand Down
2 changes: 1 addition & 1 deletion test/abstract_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* Testing dependencies
*/
import { assert } from 'chai'
import sinon, { SinonSpy } from 'sinon'
import sinon from 'sinon'
import fs from 'fs'
import levelStore from 'mqtt-level-store'
import Store from '../src/lib/store'
Expand Down
34 changes: 34 additions & 0 deletions test/client_mqtt5.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1136,6 +1136,40 @@ describe('MQTT 5.0', () => {
},
)

it('suback handling error codes', function _test(t, done) {
serverThatSendsErrors.listen(ports.PORTAND117)

serverThatSendsErrors.once('client', (serverClient) => {
serverClient.on('subscribe', (packet) => {
serverClient.suback({
messageId: packet.messageId,
granted: packet.subscriptions.map((e) => 135),
})
})
})

const client = mqtt.connect({
protocolVersion: 5,
port: ports.PORTAND117,
host: 'localhost',
})

client.subscribe('$SYS/#', (subErr) => {
client.end(true, (endErr) => {
serverThatSendsErrors.close((err2) => {
if (subErr) {
assert.strictEqual(
subErr.message,
'Subscribe error: Not authorized',
)
return done(err2 || endErr)
}
done(new Error('Suback errors do NOT work'))
})
})
})
})

it(
'server side disconnect',
{
Expand Down