-
-
Notifications
You must be signed in to change notification settings - Fork 232
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
Bump deps and fixed tests #479
Conversation
+ [email protected] + [email protected] + [email protected] + @types/[email protected] + @typescript-eslint/[email protected] + @typescript-eslint/[email protected]
Pull Request Test Coverage Report for Build 90973369
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
About your LGTM @mcollina, I don't understand if the answer to my question
Is yes it is wanted or not it should not work like that. The connection is closed and I think that that error should not be thrown by the client. The stream has been closed correctly |
tests are passing and code is ok for me. I didn’t understand your question. |
@mcollina I don't think mqtt client should throw that error when closing client. That's why my question I think that once the stream is closed using client end there should be no error thrown by the stream |
I don’t know and it does not matter for Aedes. There are a few new folks that are working on MQTT.js. |
@mcollina
I don't know if this was wanted or no. In [email protected] the only stream error thrown was 'ECONNREFUSED' but in [email protected] now it throws all this errors:
Source: https://github.com/mqttjs/MQTT.js/pull/1076/files#diff-50cfa59973c04321b5da0c6da0fdf4feR30
Here you see the commit that has changed this: https://github.com/mqttjs/MQTT.js/pull/1076/files#diff-50cfa59973c04321b5da0c6da0fdf4feL325