Fixes#2941
This adds proper exception handlers in the right places. And tests
ensure that it indeed provides nice neat logs without large
stacktraces for every amqp operation.
Unnecessary checking for subscribe permissions on topic was dropped,
as `queue.bind` does exactly the same check. Topic permissions tests
were also added, and they indeed confirm that there was no change in
behaviour.
Ideally the same explicit topic permission check should be dropped for
publishing, but it's more complicated - so for now there only a
detailed comment in the source code explaining it.
A few other things were also optimized away:
- Using amqp client to test for queue existence
- Creating queues/starting consumptions too eagerly, even if not yet
requested by client
This line is printed on every new MQTT connection which leads to very chatty logs when there is a lot of connections. Given that the way MQTT uses vhosts is generally static (once set up, always the same for all connections), I think this can be a debug message instead.
Lager strips trailing newline characters but OTP logger with the default
formatter adds a newline at the end. To avoid unintentional multi-line log
messages we have to revisit most messages logged.
Some log entries are intentionally multiline, others
are printed to stdout directly: newlines are required there
for sensible formatting.
We have considered multiple options in preventing a split cluster
scenario when N nodes a started in parallel and are initially unaware of
each other. They all are fairly involved and run various risks, e.g.
of losing consistency for cluster members that need to rejoin a newly
discovered set of members.
A simple delay to see if there may be any peers seems to be a straightfoward
solution that would make a practical difference.
In the future consistent client ID tracking should be a feature the user
can opt out of because it tilts MQTT plugin potentially to far towards
C on the consistency/availability range.
Pair: @kjnilsson
To avoid blocking when registering or unregistering a client id. This is
ok as informing the current connection holder of the client id is
already async. This should be more scalable and provide much better MQTT
connection setup latency.
Changes initial_state/4 to initial_state/5 to add the peer
address that needs to be provided by Web MQTT. This function
was only used locally and by Web MQTT.
This gives a greater chance to the Last Will message to be delivered
because it will implicitely block `rabbit_mqtt_reader` while the
server-side AMQP channel processes the message.
Without this, `rabbit_mqtt_reader` asks the Last Will message to be
sent asynchronously and immediately closes the AMQP connection. The
server-side AMQP channel might thus try to query an already closed
connection.
Fixes#146.
[#150162950]
(cherry picked from commit a2eb8c1be8c52d476ee9ea0ff08ef381a2ca326c)
This gives a greater chance to the Last Will message to be delivered
because it will implicitely block `rabbit_mqtt_reader` while the
server-side AMQP channel processes the message.
Without this, `rabbit_mqtt_reader` asks the Last Will message to be
sent asynchronously and immediately closes the AMQP connection. The
server-side AMQP channel might thus try to query an already closed
connection.
Fixes#146.
[#150162950]
Lager sink parse-transform allows us to use a fake module name
to select sinks.
It's more convenient that to use a helper function for every module.
[#149634975]
Plugin handles exit signal coming from the AMPQ core,
logs, and closes the client connnection (instead of letting
the whole process tree crash with scary log messages).
References rabbitmq/rabbitmq-server#505
check_resource_access used to be called with
the MQTT topic as resource name and kind = topic.
It makes more sense now to call check_topic_access
with the exchange as resource name, kind = topic,
and routing key in the context.
References rabbitmq/rabbitmq-server#505