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.

Sign up for GitHub

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

Jump to bottom

[Bug]: WebSocket not completely disconnected #1817

Closed
ogis-yamazaki opened this issue Mar 15, 2024 · 3 comments · Fixed by #1820
Closed

[Bug]: WebSocket not completely disconnected #1817

ogis-yamazaki opened this issue Mar 15, 2024 · 3 comments · Fixed by #1820
Labels
bug

Comments

@ogis-yamazaki
Copy link
Contributor

MQTTjs Version

5.4.0

Broker

mosquitto v2.0.18

Environment

Browser

Description

I executed the mqtt.Client#end() function with the force option on the Websocket connection.
But it doesn't seem to be actually disconnected.

It happens in the browser, not in the Node program

The following sample code did not send a Will message.
Not when the client disconnects, Will message was sent when the broker disconnects after keep_alive seconds.

Minimal Reproduction

u1 user is monitoring Will message arrival while u2 is forcibly disconnecting

u1

<!DOCTYPE html>
<html>
<head>
    <meta charset="utf-8">
    <meta http-equiv="Pragma" content="no-cache">
    <meta http-equiv="Cache-Control" content="no-cache">
    <title>user 1</title>
</head>
<body>
    <script src="https://unpkg.com/mqtt@5.4.0/dist/mqtt.js"></script>
    <script>
        const connectOptions = {
            keepalive: 300,
            clean: true,
            clientId: 'u1',
            reconnectPeriod: 0
        }

        const uri = 'ws://127.0.0.1:8080'
        const client = mqtt.connect(uri, connectOptions);

        client.on('message', (topic, message, packet) => {
            console.warn(`topic=${topic}, qos=${packet.qos}, pid=${packet.messageId}, message=${message}`)
        })

        client.once('connect', (packet) => {
            console.info(`connect event. packet = ${JSON.stringify(packet)}`)
            client.subscribe('t1', { qos: 1 }, (err, granted) => {
                if (!err) {
                    console.info(`subscribed. granted:${JSON.stringify(granted)}`)
                }
            })
        });
    </script>
</body>
</html>

u2

<!DOCTYPE html>
<html>
<head>
    <meta charset="utf-8">
    <meta http-equiv="Pragma" content="no-cache">
    <meta http-equiv="Cache-Control" content="no-cache">
    <title>user 2</title>
</head>
<body>
    <script src="https://unpkg.com/mqtt@5.4.0/dist/mqtt.js"></script>
    <script>
        const num = Date.now()
        const connectOptions = {
            keepalive: 60, // 60 sec
            clean: true,
            clientId: 'u2',
            reconnectPeriod: 0
        }
        connectOptions.will =  {topic: 't1', qos: 1, payload: `u2 is dead. ${num}`}

        const uri = 'ws://127.0.0.1:8080'
        const client = mqtt.connect(uri, connectOptions);

        client.on('close', () => {
            console.info(`close event`)
        });
        client.on('error', (err) => {
            console.info(`error event. err = ${err}`)
        });
        client.on('end', () => {
            console.info(`end event`)
        });

        client.once('connect', (packet) => {
            console.info(`connect event. packet = ${JSON.stringify(packet)}`)
            setTimeout(() => {
                console.warn(`force disconnect`)
                client.end(true)
            }, 5000) // after 5sec
        });
    </script>
</body>
</html>

environment

  • mosquitto v 2.0.18
  • mqtt.js 5.4.0

Debug logs

--
@robertsLando
Copy link
Member

robertsLando commented Mar 15, 2024 •
edited
Loading

@ogis-yamazaki is this a regression? Does this happens only in latest version or also previous one? If so could you detect which version caused the issue? I also suggest you to enable debug logs and see what happens, the will message should be sent from broker when it detects an unexpected close from client, this means for some reason this is not closing the socket:

MQTT.js/src/lib/connect/ws.ts

Line 179 in d86947f

socket.close()

And that is triggered when the stream is closed:

MQTT.js/src/lib/BufferedDuplex.ts

Line 77 in d86947f

this.proxy.end(callback)

Try to debug that and let me know

robertsLando added a commit that referenced this issue Mar 15, 2024
@robertsLando
fix(browser): force closing client doesn't destroy websocket correctly
Loading
Loading status checks…
d955591 
Fixes #1817
@robertsLando robertsLando mentioned this issue Mar 15, 2024
Merged
robertsLando added a commit that referenced this issue Mar 15, 2024
@robertsLando
fix(browser): force closing client doesn't destroy websocket correctly (

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
GPG key ID: B5690EEEBB952194
Verified
Learn about vigilant mode
Loading
Loading status checks…
f9b1204 
#1820)

Fixes #1817
@robertsLando
Copy link
Member

Seems the issue was in introduced with BufferedDuplex in v5.3.0 so it affects all versions up to 5.4.0

@ogis-yamazaki
Copy link
Contributor Author

Thank you for fixing the bug!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(browser): force closing client doesn't destroy websocket correctly mqttjs/MQTT.js
2 participants
@robertsLando @ogis-yamazaki