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

MQTTS connection error on nodejs >= 8.6.0 #699

Closed
redboltz opened this issue Oct 16, 2017 · 3 comments · Fixed by #700
Closed

MQTTS connection error on nodejs >= 8.6.0 #699

redboltz opened this issue Oct 16, 2017 · 3 comments · Fixed by #700

Comments

@redboltz
Copy link
Contributor

After updating nodejs from 8.5.0 to 8.6.0, MQTTS connection becomes failure.

The reason is the commit nodejs/node@ee157e5a7f
See https://github.com/nodejs/node/blob/master/doc/changelogs/CHANGELOG_V8.md#8.6.0

Here is the document.
https://nodejs.org/api/tls.html#tls_tls_connect_options_callback

"path Creates unix socket connection to path. If this option is specified, host and port are ignored."

I analyzed the problem. And
https://github.com/mqttjs/MQTT.js/blob/master/lib/connect/index.js#L60 adds path member.

And the variable opt that contains path member passed to https://github.com/mqttjs/MQTT.js/blob/master/lib/connect/tls.js#L11

Hence host and port are ignored.

@redboltz
Copy link
Contributor Author

I realized that I passed mqtts://hostname:1234/. If I remove the last /, the connection is successfully finished. However, I think that ignoring path might be convenient. Because existing working code don't need to be changed.

So I post the PR #700.

@bengl
Copy link

bengl commented Oct 17, 2017

Perhaps MQTT should be in https://github.com/nodejs/citgm ?

@mcollina
Copy link
Member

@bengl I think so, too. However our tests are network heavy and they tend to bet too flaky to be in CITGM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants