bytebeamio / rumqtt

The MQTT ecosystem in rust
Apache License 2.0
1.57k stars 243 forks source link

MQTT.js connections hits timeout #890

Open mdrssv opened 1 month ago

mdrssv commented 1 month ago

Please describe the behavior you are expecting

mqtt.js can connect

What is the current behavior?

Websocket connection fails with:

rumqttd_1  | 2024-07-17T12:47:00.921960Z ERROR remote_link{tenant_id=None}: rumqttd::server::broker: Error while handling MQTT connect packet error=Network(KeepAlive(Elapsed(())))

Only fails with mqtt.js but not rumqttc(examples/websocket)

const mqtt = require("mqtt");

let url = process.argv[2]
let user = process.argv[3]
let pass = process.argv[4]
let topic = process.argv[5]
let msg = process.argv[6]

process.argv.forEach(function (val, index, array) {
  console.log(index + ': ' + val);
});

let client = mqtt.connect(url, {
    reconnectPeriod: 0,
    username: user,
    password: pass,
    protocolVersion: 4,
    keepalive: 15
}); // create a client

client.on('error', (err) => {
    console.log(err);
    client.end();
});

client.on('connect', () => {
    console.log('connected')
    client.publish(topic, msg, (err) => {
        console.log(err);
        //client.end();
    });
});
DEBUG='mqttjs*' node mqtt_pub.js ws://localhost:8080 user p@ssw0rd test/topic test;

Please help provide information about the failure if this is a bug. If it is not a bug, please remove the rest of this template.

Context

Hits timeout at: https://github.com/bytebeamio/rumqtt/blob/db1f261dd5cd6c69bbfd1058ba69ea8ef5f4fc38/rumqttd/src/link/remote.rs#L185

and once the first timeout is commented it'll hit here: https://github.com/bytebeamio/rumqtt/blob/db1f261dd5cd6c69bbfd1058ba69ea8ef5f4fc38/rumqttd/src/link/network.rs#L106

since self.keepalive is set to 0ns

swanandx commented 1 month ago

can you share your rumqttd config? what are you setting as connection_timeout_ms?

also, as it works for rumqttc and only fails for mqtt.js, can you verify if mqtt.js is working with other broker?

mdrssv commented 1 month ago

Certainly:

id = 0

# A commitlog read will pull full segment. Make sure that a segment isn't
# too big as async tcp writes readiness of one connection might affect tail
# latencies of other connection. Not a problem with preempting runtimes
[router]
id = 0
max_connections = 10010
max_outgoing_packet_count = 200
max_segment_size = 104857600
max_segment_count = 10
# shared_subscriptions_strategy = "random" # "sticky" | "roundrobin" ( default ) | "random"
# Any filters that match to configured filter will have custom segment size.
    # [router.custom_segment.'/office/+/devices/status']
    # max_segment_size = 102400
    # max_segment_count = 2
    # [router.custom_segment.'/home/+/devices/status']
    # max_segment_size = 51200
    # max_segment_count = 2

# name = "bridge-1"
# addr = "localhost:1883"
# qos = 0
# sub_path = "#"
# reconnection_delay = 5
# ping_delay = 5
# timeout_delay = 5
#     [bridge.connections]
#     connection_timeout_ms = 60000
#     max_payload_size = 20480
#     max_inflight_count = 500
#     dynamic_filters = true
#     [bridge.transport.tls]
#     ca = "ca.cert.pem"
#     client_auth = { certs = "test-1.cert.pem", key = "test-1.key.pem" }

# Configuration of server and connections that it accepts
[prometheus]
listen = "127.0.0.1:9042"
interval = 1

[ws.1]
name = "ws-1"
listen = "0.0.0.0:8080"
next_connection_delay_ms = 1
    [ws.1.connections]
    connection_timeout_ms = 10000
    max_client_id_len = 256
    throttle_delay_ms = 0
    max_payload_size = 1073741824
    max_inflight_count = 500
    max_inflight_size = 1024
    auth = { user = "p@ssw0rd" }

[console]
listen = "0.0.0.0:3030"

# [metrics]
#     [metrics.alerts]
#     push_interval = 1
#     [metrics.meters]
#     push_interval = 1

I'm quite surpised, that connection_timeout_ms isn't used instead of keepalive in network.rs

swanandx commented 1 month ago

I'm quite surpised, that connection_timeout_ms isn't used instead of keepalive

connection_timeout_ms is for preventing DOS attacks where client just opens connection but never send connect packet. On other hand, once connection is accepted, we use keepalive value to determine when it should be dropped based on it's keep alive value set in connect packet.

mdrssv commented 1 month ago

also, as it works for rumqttc and only fails for mqtt.js, can you verify if mqtt.js is working with other broker?

mqtt.js works as expected with mosquitto